1

I want a piece of code to run as long as the thread isn't interrupted. Currently what I'm doing is this:

while(!Thread.interrupted()){
    // .. some code
    try {
        Thread.sleep(4000);
    } catch(InterruptedException ex){
        break;
    }
    // .. some more code
}

My question is: is this good practice? Is it appropriate use of interrupt?

Aviv Cohn
  • 15,543
  • 25
  • 68
  • 131
  • possible duplicate of [How can I kill a thread? without using stop();](http://stackoverflow.com/questions/5915156/how-can-i-kill-a-thread-without-using-stop) – Alexis Leclerc Sep 04 '14 at 23:52
  • @AlexisLeclerc The intention seems to be to "wake up" the thread rather than "kill" it. – Costi Ciudatu Sep 04 '14 at 23:53
  • @CostiCiudatu I wasn't clear in the question. Actually I do want to wake it up and then kill it. – Aviv Cohn Sep 04 '14 at 23:54
  • @CostiCiudatu Yeah, I just saw that. The question I linked was from another question that was marked as its duplicate, but it seems that this duplicate question has a misreplaced link. – Alexis Leclerc Sep 04 '14 at 23:56
  • @CostiCiudatu [The duplicate I'm talking about](http://stackoverflow.com/questions/4264355/interrupting-or-stopping-a-sleeping-thread) – Alexis Leclerc Sep 04 '14 at 23:57
  • Seems like a reasonable question to me. Vote to reopen. I suppose it could be a bit more specific though. – Stuart Marks Sep 05 '14 at 06:01

3 Answers3

2

Yes, I think this is the very purpose of interrupt(), so it seems like a legitimate usage.

Costi Ciudatu
  • 37,042
  • 7
  • 56
  • 92
  • Well yes, I want the thread to come out of it's sleep and finish. So is calling `interrupt` and `return`ing in the `catch` for the `InterruptedException` a good idea? – Aviv Cohn Sep 04 '14 at 23:53
  • I don't think so. Since you're actually waiting for some event to occur before you cleanup, that's a standard usage for `Object.wait()` or `Condition.await()`. But it may be useful if you provide more context for what you're trying to do: why exactly do you want your thread to sleep until it gets a termination message ? – Costi Ciudatu Sep 04 '14 at 23:57
  • @Prog I now realize that you execute that block of code every 4 seconds and you stop doing that on interruption. In this case, I think it makes perfect sense to use `interrupt` here. – Costi Ciudatu Sep 05 '14 at 00:30
  • Okay so to make sure I understand what I'm doing: The `!Thread.interrupted()` in the `while` makes the loop work as long as the thread hasn't been told to stop. And using a `catch(InterruptedException)` block to handle an `interrupt` while sleeping is standard. Correct? – Aviv Cohn Sep 05 '14 at 00:49
  • Yes, the only thing that seems a bit confusing is whether you care about what block gets executed last ("some code" or "some more code"); that's currently undetermined. Depending on where the interruption occurs (while sleeping or after sleeping) you may finish with the first or second block being the last one to execute. – Costi Ciudatu Sep 05 '14 at 01:00
  • No, it isn't! If interruption happens when the thread is sleeping, the interruption exception is cought, but the interruption flag is not set! Hence the next check of Thread.interrupted() will return false and the loop will never end! – isnot2bad Sep 05 '14 at 09:53
  • @isbot2bad As you can see in the code above, the `catch` does `break` in case of an exception. – Aviv Cohn Sep 05 '14 at 11:18
1

Conceptually this is on the right track. There are some details that could be improved, though.

What's right about this is that the thread interruption mechanism is being used to tell the thread to do something else, and the thread can respond how it pleases at a time of its choosing. In this case, catching InterruptedException from Thread.sleep() and breaking out of the loop is an entirely reasonable thing to do. It's certainly much preferable to what is usually done, which is to ignore the exception entirely. (This is usually wrong.)

What might be an issue is that the loop condition also checks whether the thread has been interrupted. This might be a problem, depending on what some code and some more code are doing. Your system might be left in different states depending on which chunks of code have been executed when the interrupt is detected.

Unless one iteration of your loop runs for a really long time (aside from the sleep), it's usually only necessary for there to be a single interruption point in a loop. In this case, if the thread is interrupted while it's doing some code processing, an InterruptedException will be generated immediately upon the call to Thread.sleep(). So you could just change the loop condition to while (true) and have the catch-block break out of the loop. (If you do this, you should also reassert the interrupt bit; see below.)

Alternatively, if you only want to check for interrupts at the top of the loop, you could do this:

while (!Thread.interrupted()) {
    // .. some code
    try {
        Thread.sleep(4000L);
    } catch (InterruptedException ie) {
        Thread.currentThread().interrupt();
    }
    // .. some more code
}

This arranges there to be only a single exit point from the loop, which might makes the code easier to reason about.

Note the technique used here is to reassert the interrupt bit on the thread after having caught InterruptedException. The general rule about handling interrupts is that either the interrupt bit should be reasserted, or an InterruptedException should be propagated. If neither is done, then the calling code might end up blocked in a subsequent wait() call, with no knowledge that it had ever been interrupted. That's usually an error.

Stuart Marks
  • 127,867
  • 37
  • 205
  • 259
  • "...the thread can respond how it pleases..." One thing I always worry about is, what happens if the interrupt occurs while my thread is in a library routine. In that case, the state of library objects will depend entirely on how the library author pleases. Can I generally trust library authors not to do anything worse after an interrupt than they would do if the call failed for some other reason? I don't recall ever seeing a library that explicitly documents its interrupt handling behavior. – Solomon Slow Sep 05 '14 at 12:37
  • @jameslarge It depends on the quality of the library, but yes, you're probably right to be concerned. I wouldn't be surprised if there are libraries out there that mishandle interrupts, mainly by ignoring them. This leads people to think "therefore, I need to use `Thread.stop()`" which leads to a whole new class of problems. See for example [this question](http://stackoverflow.com/questions/25489720). – Stuart Marks Sep 05 '14 at 15:03
0

Good practice is to use the newer higher level APIs such as an ExecutorService instead. You shouldn't use the lower level APIs such as threads or synchronize or wait/notify.

clay
  • 18,138
  • 28
  • 107
  • 192