4

Looking at sample code using Thread, I often encounter this retry pattern.

boolean retry = true;
while (retry) {
    try {
        myThread.join();
        retry = false;
    } catch (InterruptedException e) {
        // Nothing ...
    }
}

join() is supposed to wait forever.

If the current thread is interrupted prior or during join and thus receive an InterruptedException, does myThread actually join ?

Is this some cut-paste remnants turned into a pattern ?

diapir
  • 2,872
  • 1
  • 19
  • 26

2 Answers2

2

The other Thread did not join() your Thread in case of an InterruptedException. join() is supposed to wait forever or until the calling Thread is interrupt()-ed.

First you need to consider whether or not an InterruptedException can actually happen. There are several possibilities how to deal with it. You can:

  • Swallow it. Warning Only do so if you're sure that InterruptedException will never happen. (Are you sure? Really?)
  • Set the interrupted flag again. This is what you do if your code doesn't know how to handle it but the Thread is doing other stuff as well that might be interested in the InterruptedException.
  • Propagate it. This is the best solution if your current method doesn't know what to do with it, but maybe the caller knows.
  • Handle it. Maybe your current Thread simply wants to stop (by completing its run() method) when you received the InterruptedException.
  • Wrap it in another exception. There are two variants of this.
    • You may want to wrap it in an Assertion if the InterruptedException shouldn't have happened in the first place and clearly is an error.
    • You may want to wrap it in another exception if your method is bound to a specific contract that doesn't allow InterruptedException.

In any case, I strongly recommend to log() the InterruptedException in some way. If you don't log it, and it happens, the program's behavior might be difficult to understand without that log information about the InterruptedException.

Swallowing it

Warning Only swallow it if you are absolutely sure that this is okay in your case. You have been warned.

public static void joinThread(final Thread thread) {
    while (true)
        try {
            thread.join();
            return;
        } catch (final InterruptedException e) {
            Logger.getGlobal().log(Level.ERROR, "Unexpected InterruptedException", e);
        }
}

As a variant of this, you can simply ignore it without a retry.

public static void joinThread(final Thread thread) {
    try {
        thread.join();
    } catch (final InterruptedException e) {
        Logger.getGlobal().log(Level.ERROR, "Unexpected InterruptedException", e);
    }
}

Setting the interrupted flag again.

This is what you do if the Thread is doing other stuff as well, your code doesn't know how to deal with the InterruptedException but the other code does.

public static void joinThread(final Thread thread) {
    try {
        thread.join();
    } catch (final InterruptedException e) {
        Logger.getGlobal().log(Level.INFO, "Unexpected InterruptedException, resetting flag", e);
        Thread.currentThread().interrupt();
    }
}

But be warned! This only is a good idea if other code really knows what to do. If the other code does the same thing, and the stuff is in a loop (which is often the case for Threads), you've just turned your nice blocking code into busy-waiting garbage because the interrupted flag never gets cleared properly. While this solution is advertised as the best or true solution in many blog articles, it's nonsense as long as there isn't any code that knows how to deal with InterruptedException and actually keeps the flag cleared instead of resetting it.

Propagating it

This is the best solution in case the method itself doesn't know how to handle it, but the caller might know.

public static void joinThread(final Thread thread) throws InterruptedException {
    try {
        thread.join();
    } catch (final InterruptedException e) {
        Logger.getGlobal().log(Level.FINE, "Got InterruptedException, propagating it to the caller", e);
        throw e;
    }
}

In case the caller does something nice with it, you can remove the unnecessary parts like logging and re-throwing.

Handling it.

In case the method itself knows that when interrupted, it has to do something, it can simply catch the exception and do whatever is expected.

public static void joinThread(final Thread thread) {
    try {
        thread.join();
    } catch (final InterruptedException e) {
        Logger.getGlobal().log(Level.FINE, "Got InterruptedException, handling it", e);
        // ...whatever...
    }
}

Wrapping it.

If the exception was not supposed to happen, because the application does not support interruption, and Thread.interrupt() or ThreadGroup.interrupt() shouldn't have been called in the first place, turning the exception into an AssertionError is the way to go. There are two possibilities to do so.

// Only throws AssertionError if assertions are enabled.
public static void joinThread(final Thread thread) {
    try {
        thread.join();
    } catch (final InterruptedException e) {
        Logger.getGlobal().log(Level.ERROR, "Unexpected InterruptedException", e);
        assert false : e;
    }
}

// Always throws AssertionError.
public static void joinThread(final Thread thread) {
    try {
        thread.join();
    } catch (final InterruptedException e) {
        Logger.getGlobal().log(Level.FATAL, "Unexpected InterruptedException", e);
        throw new AssertionError(e, "Unexpected Thread interruption.");
    }
}

If the InterruptedException is to be propagated but the caller doesn't support the exception and cannot be changed, you can wrap it into an exception that is supported. Whether or not this works and is a good idea really depends much on the caller.

public static void joinThread(final Thread thread) {
    try {
        thread.join();
    } catch (final InterruptedException e) {
        Logger.getGlobal().log(Level.ERROR, "Unexpected InterruptedException", e);
        throw new SomeException(e, "Unexpected Thread interruption.");
    }
}

Final notes

There has been some discussion that swallowing is always bad. That's not true. If you have 100 lines of code and do some Thread.sleep() somewhere for some good reason, swallowing is usually fine. As always, it depends. In the end, the semantics of Thread.interrupt() are not so different from SIGHUP, and SIGHUP is usually ignored.

There also has been some discussion whether turning it into an AssertionError is a good idea. AssertionError is an unchecked Throwable, not even an Exception, which means the compiler doesn't force code to deal with it and most code isn't written to deal with it - that's intentional, because by using AssertionError we say here's an error that was so unexpected that the code doesn't know how to deal with it and therefore wants to stop. Whether AssertionError terminates the VM depends on which Thread(s) would be taken down due to the AssertionError being thrown up to the UncaughtExceptionHandlers of the affected Threads.

Christian Hujer
  • 17,035
  • 5
  • 40
  • 47
  • 1
    I don't know of any legitimate case where it would be constructive to ignore an `InterruptedException`. An answer advocating that idea should definitely contain strong justification for something which is otherwise considered a notorious anti-pattern in Java. – Marko Topolnik Dec 08 '14 at 12:24
  • 1
    ...and to rethrow it as a non-checked throwable such as AssertionError is pretty much as bad as ignoring it. – aioobe Dec 08 '14 at 12:27
  • @aioobe And not just unchcecked: not even a subtype of `Exception`. That truly sounds like an extreme measure with the potential of shutting down the entire JVM. – Marko Topolnik Dec 08 '14 at 12:29
  • While the objections to both methods are correct from an academical perspective, the only way how to trigger an `InterruptedException` is to call `Thread.interrupt()` or `ThreadGroup.interrupt()`. If that's not going to happen in the application, why bother about it. But your objections are right for a more generic answer, I will update it. – Christian Hujer Dec 08 '14 at 12:59
  • About `unchecked`, if I may quote Robert C. "Uncle Bob" Martin: "The experiment with checked exceptions is over, it has failed." I'm not as extreme as Robert C. Martin, but `AssertionError` is really intentional here. If it is not supposed to happen but happens and no code knows to deal with it, that's what `assert` and `AssertionError` have been invented for. – Christian Hujer Dec 08 '14 at 13:22
  • Your answer seems to indicate that there is a way for the interrupted code to know the reason for the interruption. If you (thread 1) are sitting at a desk reading a book, and your friend (thread 2) *interrupts* you, there's no way to know if he wants to ask you if you want to go to lunch or ask which book you're reading. The only sensible thing to do is to stop reading and be observant of your environment. (You don't think "I'll ignore his interruption, because I'm not interested in lunch right now".) Also, I'm not sure how the discussion on checked vs. unchecked exceptions is relevant. – aioobe Dec 09 '14 at 13:23
  • If I'm a process and I get SIGHUP, I'll either ignore it or I'll precisely know what to do - what I'm defined to do in case of SIGHUP. We're still talking of pre-programmed code with pre-programmed decisions. – Christian Hujer Dec 09 '14 at 13:53
  • Well, I have over 60 different signals on my system. A signal conveys much more semantics than a Java interruption. Since you brought in OS-level analogy, why not compare a Java interrupt with a processor interrupts? – aioobe Dec 09 '14 at 15:03
  • That's a good point, which is why I do not compare `Thread.interrupt()` to signals in general but to just one specific signal, `SIGHUP`. Processor interrupts are pretty asynchronous by nature, the code executed by them is not known to the code that is being interrupted, the code that is being interrupted even doesn't know it's being interrupted (until it explicitly tries to detect this). Therefore I would compare processor interrupts to a high priority thread working on priority queues, not to `Thread.interrupt()`. – Christian Hujer Dec 09 '14 at 19:23
1

If the myThread.join() call is interrupted, the thread is not joined. myThread will keep running, unaware of the fact that the thread that called joined has been interrupted.

I have not seen this pattern myself, and I'd say it's a terrible pattern. You should never ignore an InterruptedException this way. See this post for a description on how to properly handle InterruptedExceptions.

Community
  • 1
  • 1
aioobe
  • 413,195
  • 112
  • 811
  • 826