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?

Solution 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.

Solution 2

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.

Solution 3

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.

Solution 4

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.

Solution 5

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.

View more solutions

Comments

[Will]

The sleep() only gets interrupted if someone interrupts it; in your handler, you just demote the checked exception into an unchecked one. Ouch.

[Will]

Interrupting is graceful signalling, its not pausing or aborting the thread.

[Bob Cross]

@Will, that's the point: I accept the interrupt and then force the interrupt to continue. If I simply had a "throws InterruptedException" clause on the method, anyone with a "catch (Exception e) {}" block would eat the exception. Sleep gets interrupted on a control-C: if you eat the exception, your thread won't stop.

[brady]

Exactly. It's very important that code checks for interruption if you intend to use it in a cancellable task. There's nothing "evil" about interrupt; it is still depended on heavily by experts like Doug Lea.

[Bob Cross]

@erickson, Thanks - I added the link to the listing just now. I never realized that they'd put the code samples up on the JCIP site. That should come in very handy!

[Brian Ensink]

Ah yes, my mistake. Stop is the evil one. Thanks!

[artbristol]

I would argue that your code snippet still eats the exception. If the code is called from some other method, which then goes on to perform a long-running operation, the thread won't finish. It's only ok to do what you suggest if Thread.currentThread().interrupt(); is basically at the end of a Runnable's run method. If the code could be called from anywhere, it should throw an unchecked exception to force the thread to die.

[Bob Cross]

@artbristol, I changed the language to emphasize the importance of the interrupt status. Also discussed how throwing an unchecked exception is worse than useless.

[artbristol]

@BobCross finally blocks will still run; synchronized locks will be dropped; your 'jerk' comment is noted and my downvote stays

[Bob Cross]

@artbristol, your opinion is noted - you're arguing with Brian Goetz, not me. Ref: JCIP Chap 7.1 page 143.

[artbristol]

@BobCross without the book in front of my I can't check, but you might be misinterpreting. I'm saying you need to preserve the interrupt status - always - but that you should probably also throw an exception, because otherwise the calling code is going to merrily carry on (unless it happens to check the interrupted status, and when did you ever do that, except as part of a thread pool implementation?).

[Chad N B]

@artbristol I agree with Bob; however, I also support your idea that it is preferable, if possible, to throw InterruptedException too. Personal opinion, I think it is less likely to introduce bugs by throwing an exception and makes the intent clearer. But your argument that "the calling code is going to merrily carry on" I think does not hold water because if the caller itself is a long running task, it should be checking the interrupt status. Otherwise you are saying that does not manage its own cancellation policy, which is risky.

[artbristol]

@ChadNB But calling code doesn't necessarily know it's in a long-running task; it could be any old library, invoking a callback, or it could be some framework.

[Bob Cross]

@ChadNB, you are right: the long running tasks should be checking their interrupt status. If my code is being called by their code, I may not have the option of throwing the exception: if I can't change my method signature, for example. However, I can pass the message up of "we've received an interrupt signal: you, the calling code, should really figure out how to exit cleanly." See Listing 7.7 (and the surrounding text) in JCIP for related discussion.