Is Thread.interrupt() evil?

A teammate made the following claim:

"Thread.interrupt() is inherently broken, and should (almost) never be used".

I am trying to understand why this is the case.

Is it a known best practice never to use Thread.interrupt()? C开发者_Python百科an you provide evidence why it is broken / buggy, and should not be used for writing robust multithreaded code?

Note - I am not interested in this question if it's "pretty" from a design preservative. My question is - is it buggy?

Short version:

Is it a known best practice never to use Thread.interrupt()?

No.

Can you provide evidence why it is broken / buggie, and should not be used for writing robust multithreaded code?

The opposite is true: it is critical for multithreaded code.

See Listing 7.7 in Java Concurrency in Practice for an example.

Longer version:

Around here, we use this method in one specific place: handling InterruptedExceptions. That may seem a little strange but here's what it looks like in code:

try {
    // Some code that might throw an InterruptedException.  
    // Using sleep as an example
    Thread.sleep(10000);
} catch (InterruptedException ie) {
    System.err.println("Interrupted in our long run.  Stopping.");
    Thread.currentThread().interrupt();
}

This does two things for us:

1. It avoids eating the interrupt exception. IDE auto-exception handlers always provide you with something like ie.printStackTrace(); and a jaunty "TODO: Something useful needs to go here!" comment.
2. It restores the interrupt status without forcing a checked exception on this method. If the method signature that you're implementing does not have a throws InterruptedException clause, this is your other option for propagating that interrupted status.

A commenter suggested that I should be using an unchecked exception "to force the thread to die." This is assuming that I have prior knowledge that killing the thread abruptly is the proper thing to do. I don't.

To quote Brian Goetz from JCIP on the page before the listing cited above:

A task should not assume anything about the interruption policy of its executing thread unless it is explicitly designed to run within a service that has a specific interruption policy.

For example, imagine that I did this:

} catch (InterruptedException ie) {
    System.err.println("Interrupted in our long run.  Stopping.");
    // The following is very rude.
    throw new RuntimeException("I think the thread should die immediately", ie);
}

I would be declaring that, regardless of other obligations of the rest of the call stack and associated state, this thread needs to die right now. I would be trying to sneak past all the other catch blocks and state clean-up code to get straight to thread death. Worse, I would have consumed the thread's interrupted status. Upstream logic would now have to deconstruct my exception to try to puzzle out whether there was a program logic error or whether I'm trying to hide a checked exception inside an obscuring wrapper.

For example, here's what everyone else on the team would immediately have to do:

try {
    callBobsCode();
} catch (RuntimeException e) { // Because Bob is a jerk
    if (e.getCause() instanceOf InterruptedException) {
        // Man, what is that guy's problem?
        interruptCleanlyAndPreserveState();
        // Restoring the interrupt status
        Thread.currentThread().interrupt();
    }
}

The interrupted state is more important than any specific InterruptException. For a specific example why, see the javadoc for Thread.interrupt():

If this thread is blocked in an invocation of the wait(), wait(long), or wait(long, int) methods of the Object class, or of the join(), join(long), join(long, int), sleep(long), or sleep(long, int), methods of this class, then its interrupt status will be cleared and it will receive an InterruptedException.

As you can see, more than one InterruptedException could get created and handled as interrupt requests are processed but only if that interrupt status is preserved.

The only way I'm aware of in which Thread.interrupt() is broken is that it doesn't actually do what it seems like it might - it can only actually interrupt code that listens for it.

However, used properly, it seems to me like a good built-in mechanism for task management and cancellation.

I recommend Java Concurrency in Practice for more reading on the proper and safe use of it.

The main problem with Thread.interrupt() is that most programmers don't know about the hidden pitfalls and use it in the wrong way. For example, when you handle the interrupt, there are methods which clear the flag (so the status gets lost).

Also, the call will not always interrupt the thread right away. For example, when it hangs in some system routine, nothing will happen. In fact, if the thread doesn't check the flag and never calls a Java method which throws InterruptException, then interrupting it will have no effect whatsoever.

No, it's not buggy. It actually is the basis of how you stop threads in Java. It's used in the Executor framework from java.util.concurrent - see the implementation of java.util.concurrent.FutureTask.Sync.innerCancel.

As for failure, I've never seen it fail, and I've used it extensively.

One reason not mentioned is that the interrupt signal can be lost which makes invoking the Thread.interrupt() method meaningless. So unless your code in the Thread.run() method is spinning in a while loop the outcome of calling Thread.interrupt() is uncertain.

I noticed that when in thread ONE I execute DriverManager.getConnection() when there is no database connection available (say server is down thus finally this line throws SQLException) and from the thread TWO I explicitely call ONE.interrupt(), then both ONE.interrupted() and ONE.isInterrupted() return false even if placed as the first line in the catch{} block where SQLException is handled.

Of course I workarounded this issue implementing the extra semaphore but it is quite troublesome, as it is the very first such issue in my 15 years Java development.

I suppose it's because of bug in com.microsoft.sqlserver.jdbc.SQLServerDriver. And I investigated more to confirm that the call to the native function consumes this interruption in all cases it trhows its own, but preserves it when succeeded.

Tomek

P.S. I found the analogous issue.

P.P.S I enclose a very short example of what I'm writting above. The registered class can be found in sqljdbc42.jar. I found this bug in classes built on 2015-08-20 then I updated to the newest version available (from 2017-01-12) and the bug still exists.

import java.sql.*;

public class TEST implements Runnable{
    static{
        try{
//register the proper driver
           Class.forName("com.microsoft.sqlserver.jdbc.SQLServerDriver");
        }
        catch(ClassNotFoundException e){
            System.err.println("Cannot load JDBC driver (not found in CLASSPATH).");
        }
    }

    public void run() {
        Thread.currentThread().interrupt();
        System.out.println(Thread.currentThread().isInterrupted());
//prints true
        try{
            Connection conn = DriverManager.getConnection("jdbc:sqlserver://xxxx\\xxxx;databaseName=xxxx;integratedSecurity=true");
        }
        catch (SQLException e){
            System.out.println(e.getMessage());
        }
        System.out.println(Thread.currentThread().isInterrupted());
//prints false
        System.exit(0);
    }

    public static void main(String[] args){
        (new Thread(new TEST())).start();
    }
}

If you pass something completely incorrect, as "foo", to the DriverManager.getConnection(), you will obtain the message "No suitable driver found for foo", and the second printout will be still true as one would expect. But if you pass the correctly built string but, say, your server is down or you lost your net connection (that can generally occurr in the production environment), you will see the java.net socket timeout error printout and the thread's interrupted() state is LOST.

THe problem is not that the implementation is not buggy but rather your thread is in an unknown state when it gets interrupted and this can lead to unpredictable behavior.