views:

405

answers:

3

In Tomcat, I wrote a ServletContextListener which will start an ExecutorService during startup and terminate it when it is unloaded.

I am following the example in the javadoc for ExecutorService

public void contextDestroyed( ServletContextEvent sce )
{
    executor.shutdown();
    try
    {
        executor.awaitTermination( 50, TimeUnit.SECONDS );
    }
    catch( InterruptedException ie )
    {
        Thread.currentThread().interrupt();
    }
}

My question is should I propagate the InterruptedException in the contextDestroyed() method ?

+2  A: 

I would say no. The contextDestroyed method is called by the container as a notification that the context is about to be torn down, it's not asking your permission. Also, the Javadoc does not define what happens if you throw an exception from it, so the results might be unpredictable and/or non-portable.

What I would do is call executor.shutdownNow() inside the catch block to forcibly terminate the executor (i.e. "you had your chance, now stop").

skaffman
If it is "unpredictable", what's the worst thing that could happen if an exception is thrown ?
Jacques René Mesrine
+1 spot-on with the shutDownNow() recommendation. Absorbing the exception and setting the flag here is really the right thing to do.
Tim Bender
Worst case, the code executing that callback doesn't catch it and a thread crashes.More importantly though, think about what the exception coming from "awaitTermination()" indicates. All the exception means is that the thread couldn't continue to wait for all threads to finish because it was interrupted by another thread, indicating that it should terminate. If you hadn't made a blocking call, the interrupt flag would have been set in the background and you wouldn't even know about it. So the right thing here is to set the flag, is terminate the executor and set the flag.
Tim Bender
@Tim I don't understand what you mean by "setting the flag". Do you mean the Thread interrupt flag ?
Jacques René Mesrine
+1  A: 

What you have in your code sample (re-interrupt the current thread) is exactly what I would recommend. Something in Tomcat outside your own code sent the original interrupt, so let Tomcat have a chance to handle it.

I don't know what Tomcat will do with the InterruptedException. It's undefined. But Tomcat initiated the interrupt and Tomcat owns the thread that the contextDestroyed(...) method is running in. The general principle from "Java Concurrency in Practice" that applies here is: the creator of the thread is responsible for handling thread life-cylce issues.

Handling a interrupt is definitely a life-cycle issue.

Steve McLeod
Do you know what Tomcat will do with the InterruptedException ?
Jacques René Mesrine
A: 

I agree with Steve, resetting the interrupt flag gives code outside your control a chance to react to the event.

tempus-fugit offers a convieance method to do this for you, along with an explict timeout exception if things take too long.

waitOrTimeout(shutdown(executor), timeout);

Have a look under the concurrency section of the docs if its of interest... http://code.google.com/p/tempus-fugit/wiki/Documentation

This example demonstrates its usage, both for awaiting completion and more aggressive shutdown.

Toby