Is Thread.interrupt() evil?

Question

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()? Can 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?

Answer 1

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.