Program hangs as threads don't terminate

Question

I'm having this odd behaviour. My program hangs eventhough I'm setting a time to force terminate it. could anyone point out what this strange behaviour could be?

here is my code where I start the threads

protected void pendingTaskStarter() throws Exception {
    ExecutorService service = Executors.newFixedThreadPool(maxThreadNum);
    ArrayList<Future<Runnable>> futures = new ArrayList<Future<Runnable>>();

    System.out.println("max thread num: " + maxThreadNum);
    for(int i=0;i<maxThreadNum;i++){

        Thread t = new PendingTaskConsumer();
        Future<?> f=service.submit(t);
        futures.add((Future<Runnable>) f);  
    }

    for (Future<?> future:futures) {
        future.get(5l,TimeUnit.MINUTES); // maximum thread's life is 5min (5l <-- 5 in long )
    }   
    service.shutdownNow();

}

I am 100% sure that my program hangs somewhere in PendingTaskConsumer class based on the outputs within PendingTaskConsumer class.

Anyways codes in PendingTaskConsumer should be irrelevant as thread is supposedly forced to be terminated. My question is in what scenraios the following line does not do as its expected.

future.get(5l,TimeUnit.MINUTES);

The program is running on Linux(Ubuntu) and using openjdk-7-jdk backage ( version 1.7)

Answer 1

Well... proper exception handling seems to be missing.

/**  
 * Waits if necessary for at most the given time for the computation  
 * to complete, and then retrieves its result, if available.  
 *  
 * @param timeout the maximum time to wait  
 * @param unit the time unit of the timeout argument  
 * @return the computed result  
 * @throws CancellationException if the computation was cancelled  
 * @throws ExecutionException if the computation threw an
 * exception  
 * @throws InterruptedException if the current thread was interrupted
 * while waiting  
 * @throws **TimeoutException if the wait timed out**  
 */  
V get(long timeout, TimeUnit unit)
    throws InterruptedException, ExecutionException, TimeoutException;

If your call to future.get(5l,TimeUnit.MINUTES); expires, then an Exception is thrown and your service is never shut down.

So (one can assume) its internal threads are still running, at least the one that is executing your long running task). Seeing those are non-daemons thread (you'd have to custom the ThreadFactory of your executor for them to be), those pending threads prevent the JVM from shutting down.

If you wish to terminate the tasks forcibly, you can :

1. design your runnable task to respond to Thread.isInterrupted(), and exit when it sees that become true. (The practical effect of shutdownNow is to set the interrupted flag of your executor's thread )
2. If you can't (because your are blocked in some library's method that does not listen to interruption), then having a dameon thread seems like the only option.

But anyway, I'd put the executor's shutdown either earlier, or in a finally clause.

Answer 2

As @GPI already stated in his/her answer, cancellation won't work properly, if your tasks do not repond to thread interruption via Thread.interrupt(). See Oracle: The Java™ Tutorials - Concurrency - Interrupts or Java Executors: how can I stop submitted tasks?.

But even if your tasks do not immediately stop if the thread is interrupted (=the interruption flag is set), you can timeout your method. Just don't listen to task completion by invoking every single get() method of your Futures! That's not necessary as you are not really interested in their result. Just shutdown your ExecutorService and wait for its termination.

protected void pendingTaskStarter() throws InterruptedException {
    ExecutorService service = Executors.newFixedThreadPool(maxThreadNum);

    for (int i = 0; i < maxThreadNum; i++) {
        service.submit(new PendingTaskConsumer());
    }

    // Shutdown service.
    // This will continue to process already submitted tasks.
    service.shutdown();

    try {
        // wait at least 5 minutes for the tasks to complete
        if (!service.awaitTermination(5, TimeUnit.MINUTES)) {
            // still not done yet -> force shutdown
            // this will interrupt currently running tasks.
            service.shutdownNow();
        }
    } catch (InterruptedException ex) {
        // someone interrupted this thread. Shutdown tasks immediately
        service.shutdownNow();
        // propagate exception
        throw ex;
    }
}

So what happens here?

• First, all tasks are submitted.
• Then shutdown() is called on the ExecutorService. Note that the service will continue to execute already submitted tasks, but it will not accept new tasks.
• Now, we wait for the service to terminate by calling awaitTermination with a timeout of 5 minutes. If this method returns true, which means, that the service has terminated within time, everything is fine and we're done. But if it returns false after 5 minutes, it means that the service is still running because there are some tasks that need more time to complete.
• In that case, we shutdown the service immediately by calling shutdownNow(). Note that this method will not block! It just attempts to cancel all running tasks, usually by interrupting the threads that are currently executing them. This is exactly why your tasks should response to thread interruption properly! If they don't do this, interrupting the thread will have no effect at all!
• No matter if shutdownNow was successful - we are done and do not wait any longer. So there might still be some tasks that are running because they don't react upon thread interruption (usually because a task is either not implemented properly or it is uninterruptibly on purpose). But we simply don't care.
• Note the exception handler: When another thread interrupts the thread that is calling our method while it is waiting in method awaitTermination, an InterruptedException is thrown. We handle it by shutting down the service and re-throwing the exception! That is a good thing and makes our method a long-running-operation that is cancellable by itself!