7

I was reading about ways to end a thread in https://docs.oracle.com/javase/7/docs/technotes/guides/concurrency/threadPrimitiveDeprecation.html, which is a link I found from the question How do you kill a thread in Java?

In the first link, they first discuss using a volatile variable to signal to the thread that you want to terminate. The thread is supposed to check this variable and cease operation if the variable has a value that means cease (e.g. if it is null). So to terminate the thread, you would set that variable to null.

Then they discuss adding interrupts to help with threads that block for long periods of time. They give the following example of a stop() method that sets the volatile variable (waiter) to null and then ALSO throws an interrupt.

public void stop() {
    Thread moribund = waiter;
    waiter = null;
    moribund.interrupt();
}

I am just wondering, why would you need both? Why not ONLY use interrupt(), and then handle it properly? It seems redundant to me.

Stephen
  • 8,508
  • 12
  • 56
  • 96
  • Interrupts can come from different sources, such as the OS. In those cases you wouldn't want the thread to exit out. – lordoku Mar 28 '18 at 15:28
  • 1
    A lot of answers on that question are misleading or just uninformed bad advice. A good one is http://stackoverflow.com/q/11387729 – Nathan Hughes Mar 28 '18 at 16:45
  • 2
    @lordoku: btw OS-interrupts shouldn't be relevant here, an OS-interrupt won't interrupt threads in Java unless you go to extra trouble, see [Java Hardware Interrupt Handling](https://stackoverflow.com/q/1761676/217324) or [How to gracefully handle the sigkill signal in Java](https://stackoverflow.com/questions/2541597/how-to-gracefully-handle-the-sigkill-signal-in-java). This should strictly be about java thread interruption. – Nathan Hughes Mar 28 '18 at 16:58
  • @NathanHughes Good call. I was remembering something from college incorrectly. – lordoku Mar 28 '18 at 17:23

3 Answers3

5

(First part of this is in general, arguably I was not paying attention to the specifics of the question. Skip to the end for the part that addresses the techspec discussed in the question.)

There is no good technical reason. This is partly about human limitations and partly about confusing api design.

First consider application developers’ priority is creating working code that solves business problems. Thoroughly learning low level apis like this gets lost in the rush to get work done.

Second there’s a tendency when you’re learning things to get to a good enough state and leave it there. Things like threading and exception handling are back-of-the-book topics that get neglected. Interruption is a back of the book topic for threading books.

Now imagine a codebase worked on by multiple people with varying skill level and attention to detail, who may forget that throwing InterruptedException from wait and sleep resets the interrupt flag, or that interrupted isn’t the same as isInterrupted, or that InterruptedIoException resets the interrupt flag too. If you have a finally block that catches IOException, you may miss that InterruptedException is a subclass of IOException and you could be missing out on restoring the interrupt flag. Probably people in a hurry decided to hell with it, I can’t count on this interrupted flag

Is it right? No.

  • The hand rolled flag doesn’t help with short circuiting wait or sleep the way interruption does.

  • the Java 5 concurrency tools expect tasks to use interruption for cancellation. Executors expect tasks submitted to them to check for interruption in order to quit gracefully. Your tasks may use other components, like a blocking queue. That queue needs to be able to respond to interruption, the thing using it needs to be aware of the interruption. The handrolled flag doesn’t work for this since the java 5 classes can’t know about it.

Having to use interruption because you’re using tools that expect it, but not having confidence in the flag value due to unmanageable technicalities, would lead to this kind of code.

(end of rant, now actually responding to the specific techspec example)

OK, looking at this techguide article in particular. Three things stand out:

1) it's not making use of interruption at all, except to cut the sleep time short. Then it just squelches the exception, and doesn't bother to restore the flag or check it. You could use the InterruptedException to terminate by catching it outside the while loop, but that's not what this does. This seems like a strange way to do this.

2) as the example is fleshed out it becomes clear the flag is being used to turn the waiting on and off. Somebody might use interruption for this but it's not idiomatic. So having a flag is ok here.

3) this is a toy example in a techguide. Not all the Oracle content is as authoritative as the techspecs or the API documentation. Some tutorials have misstatements or are incomplete. It might be the reason the code was written like this was that the author figured readers would not be familiar with how interruption worked and it was better to minimize its usage. Technical writers have been known to make choices like that.

If I rewrote this to use interruption I would still keep the flag; I'd use interrupt to terminate, and use the flag for suspend/resume functionality.

Nathan Hughes
  • 94,330
  • 19
  • 181
  • 276
  • Sure, but this was part of an official Java doc, not some corporate code, so I figured there must be a good reason to do both. – Stephen Mar 28 '18 at 18:36
1

Please see this documentation. Your thread should check thread.isInterrupted() status, for example:

Thread myThread = new Thread()  {
    @Override
    public void run() {
        while (!this.isInterrupted()) {
            System.out.println("I'm working");
            try {
                Thread.sleep(1000);
            } catch (InterruptedException e) {
                //We would like also exit from the thread
                return;
            }
        }
    }
};

And when you would like to stop the thread, you should invoke

myThread.interrupt();

Besides we can use static method Thread.interrupted() that also checks the status but after that, the method clears it and you have to invoke again myThread.interrupt() to set the status again. But I don't recommend to use Thread.interrupted() method.

This approach helps gracefully stop the thread.

So I also do not see any reasons to use an additional volatile variable.

The same behavior can be reached via additional volatile flag as in @lexicore's answer, but I think it is redundant code.

statut
  • 849
  • 5
  • 14
  • 1
    This does not actually answer the question. – lexicore Mar 28 '18 at 15:36
  • Agreed there is no good reason to have a volatile variable. this is more important as of java 5 with concurrency utilities like executors which assume the tasks submitted to them are well-behaved wrt interruption. in addition I might suggest this could be more strongly worded to steer people away from using Thread.interrupted. +1 – Nathan Hughes Mar 28 '18 at 17:13
  • I don't see the problem if you replace `this.isInterrupted()` with `this.interrupted()` in this example code. You don't need the flag to be reset because the loop will end and the thread will terminate. Besides, at least according to https://docs.oracle.com/javase/tutorial/essential/concurrency/interrupt.html isInterrupted() is intended for one thread to query the status of another, not for a thread to query its own status. – Stephen Mar 28 '18 at 17:31
  • @Stephen: in the specific example it works, but in general the problem is it's error-prone, resetting the flag unnecessarily, people may pick this because it's easy to use (no Thread.currentThread() stuff) and be oblivious about the side-effect. And it's ok for a task to check its own thread's status with Thread.currentThread().isInterrupted(). – Nathan Hughes Mar 28 '18 at 17:49
  • @NathanHughes well neither isInterrupted() nor interrupted() require the Thread.currentThread() stuff. Anyway I guess I'll take your word for it that it can be bad to reset the flag unnecessarily. – Stephen Mar 28 '18 at 18:03
  • @NathanHughes so just to make sure I have this right, the reason we need the isInterrupted() case at all (rather than only catching InterruptedException) is in case the interruption is received in the split second after the catch block? Otherwise I think we could have while(true) (since Thread.sleep() also checks interrupted()). Though it would become necessary again if we removed the call to Thread.sleep() so perhaps it would be safest to keep it even if that weren't possible. – Stephen Mar 28 '18 at 18:07
  • @Stephen: better example is when your code uses other objects that use interruption to cancel (java.util.concurrent classes for example). You could write your own reusable object. If it used interrupted it could clear the flag, then the code using that object might not be aware the interrupt ever happened – Nathan Hughes Mar 28 '18 at 18:16
  • @NathanHughes I don't understand, can you elaborate? I was basically trying to confirm why we don't only catch InterruptedException, using neither interrupted() nor isInterrupted(). – Stephen Mar 28 '18 at 18:47
  • @Stephen some threads might not have methods that throw InterruptedException. Even if they have these methods someone can delete them and then your thread will not be able to be interrupted. – statut Mar 28 '18 at 18:53
  • @NathanHughes I'm coding some of this stuff up and noticed one downside of using isInterrupted() instead of interrupted() - you'll have to make sure that your class extends Thread instead of Runnable, which limits your flexibility in some ways although it isn't too bad and might still be worth it (I don't have enough experience to know). – Stephen Mar 28 '18 at 19:34
  • @Stephen: no, you don't have to extend Thread. Use Thread.currentThread().isInterrupted(). It is more awkward and is probably why people choose interrupted instead. – Nathan Hughes Mar 28 '18 at 19:49
0

@NathanHughes is completely right, but I am going to rephrase his long answer into few words: third-party code.

This is not just about "dumb junior developers", — at some point in application's life you will be using lots of third-party libraries. Those libraries will not gentlemanly respect your assumptions about concurrency. Sometimes they will silently reset interruption flag. Using separate flag solves that.

No, you can not get rid of third-party code and call it a day.

Suppose, that you are writing a library, — for example, ThreadPoolExecutor. Some of the code inside your library needs to handle interruption… Or even unconditionally reset the interruption flag. Why? Because previous Runnable is done, and a new Runnable is on the way. Unfortunately, at that point there may be a stale interruption flag, that was aimed… wait, whom was it for again? It could have been addressed for previous Runnable or for new (not yet running) Runnable, — there is no way to tell. And this is why you add isCancelled method to FutureTask. And unconditionally reset interruption flag before executing new Runnable. Sounds familiar?

Thread#interrupt is completely detached from the actual work units, running on that thread, so adding an additional flag is necessary. And once you have started doing so, you have to do that all the way — up to the outermost and down to the innermost work unit. In effect, running unknown user-supplied code on your threads makes Thread#interrupted unreliable (even if Thread#interrupt still works fine!)

user1643723
  • 4,109
  • 1
  • 24
  • 48
  • I don't really understand your paragraph starting with "Suppose, that you are writing a library." If I create a library I would only need the code to worry about the current thread, not "a new Runnable [that] is on the way" – Stephen Mar 29 '18 at 15:13
  • @Stephen Your library might not be in complete control of the thread if you invoke third-party code (for example, plugins or callbacks) from your library code. If you invoke an interruptible callback, then another callback, what are you going to do if the thread gets interrupted between those callbacks? Swallow the interrupt? Crash the thread? What if one of callbacks interrupts itself (because it does not have any other way to perform cancellation)? The callback itself won't swallow it's interrupt (because it *can't* do it without race), so your library will have to deal with it. – user1643723 Mar 30 '18 at 04:50
  • @Stephen Just to clarify one thing, — have you ever written Java code outside of managed server environments? Once you write your own ThreadPoolExecutor (yes, an Executor can be a "library" too) or create an app, running outside of server container, you will have to deal with managing units of work at top level, where letting InterruptedException bubble up is no longer an option. In such case you will sometimes need to reset interruption flag without understanding, who have set it. So you will have to chose between crashing thread (not nice) or silently swallowing third-party interrupts – user1643723 Mar 30 '18 at 05:06