5

I ran into this concurrency problem which I've been scratching my head for several days.

Basically, I want my ThreadPoolExecutor to wait for all tasks (number of tasks unknown) to finish before shutting down.

public class AutoShutdownThreadPoolExecutor extends ThreadPoolExecutor{
    private static final Logger logger = Logger.getLogger(AutoShutdownThreadPoolExecutor.class);
    private int executing = 0;
    private ReentrantLock lock = new ReentrantLock();
    private final Condition newTaskCondition = lock.newCondition(); 
    private final int WAIT_FOR_NEW_TASK = 120000;

    public AutoShutdownThreadPoolExecutor(int coorPoolSize, int maxPoolSize, long keepAliveTime,
        TimeUnit seconds, BlockingQueue<Runnable> queue) {
        super(coorPoolSize, maxPoolSize, keepAliveTime, seconds, queue);
    }


    @Override
    public void execute(Runnable command) {
        lock.lock();
        executing++;
        lock.unlock();
        super.execute(command);
    }


    @Override
    protected void afterExecute(Runnable r, Throwable t) {
        super.afterExecute(r, t);
        try{
            lock.lock();
            int count = executing--;
            if(count == 0) {
                newTaskCondition.await(WAIT_FOR_NEW_TASK, TimeUnit.MILLISECONDS); 
                if(count == 0){
                    this.shutdown();
                    logger.info("Shutting down Executor...");
                }
            }
        }
        catch (InterruptedException e) {
            logger.error("Sleeping task interrupted", e);
        }
        finally{
            lock.unlock();
        }
    }
}

The intention is that the task checks for the task counter (executing), and if it equals 0, it blocks for a while, and later releases its lock so that other tasks may have the chance to execute and not shutting down the executor too soon.

However, it is not happening. All 4 threads in the executor go into the waiting state:

"pool-1-thread-4" prio=6 tid=0x034a1000 nid=0x2d0 waiting on condition [0x039cf000]
"pool-1-thread-3" prio=6 tid=0x034d0400 nid=0x1328 waiting on condition [0x0397f000]
"pool-1-thread-2" prio=6 tid=0x03493400 nid=0x15ec waiting on condition [0x0392f000]
"pool-1-thread-1" prio=6 tid=0x034c3800 nid=0x1fe4 waiting on condition [0x038df000]

If I put a log statement(supposed to slow the thread down) in the Runnable class, the problem seems to disappear.

public void run() {
    //  logger.info("Starting task" + taskName);
        try {
            //doTask();
        }
        catch (Exception e){
            logger.error("task " + taskName + " failed", e);
        }
}

The question is similar to this post Java ExecutorService: awaitTermination of all recursively created tasks

I have adopted the original poster solution and attempted to address the race condition in afterExecute() but it doesn't work.

Please help shed some light to this. Thank you.

Community
  • 1
  • 1
TommyQ
  • 501
  • 7
  • 18

2 Answers2

2

You have your tasks waiting for this newTaskCondition, but nothing ever signals this condition. So your threads all pile up, all waiting on newTaskCondition, until it times out. Additionally, blocking in afterExecute will delay the completion of the task, so it's probably not what you want to do.

If you want to keep it waiting around a little while to see if more work comes in, this functionality already exists. Simply call setKeepAliveTime to set how long to wait, and set allowCoreThreadTimeOut to ensure that all threads (including the core threads) can be terminated.

bdonlan
  • 224,562
  • 31
  • 268
  • 324
  • That's the intention. Check if there is no more task -> wait and release the lock for a while, (if there are new tasks, they should have pick up this lock) -> wakes up and re-acquires the lock -> check again if number of task is still 0 -> if yes then shut down the executor, if not just exit the task. There's no need to signal for thread to wake up in this case, I guess? – TommyQ Mar 30 '12 at 03:24
  • @TommyQ, and that's what's happening - a task runs, it sees there's no more threads, it starts waiting 120 seconds. Repeat four times, and now you have four waiting tasks. Nothing will ever wake them up until the timer expires, and if you ran out of threads because they were busy waiting 120 seconds, well, too bad. What's wrong with the built-in keepalive timer? – bdonlan Mar 30 '12 at 03:30
  • setKeepAliveTime is only applicable to excess threads? I set the corePoolsize = maxPoolsize; – TommyQ Mar 30 '12 at 03:41
0

I have found a very simple solution to my problem.

Although the number of tasks is unknown beforehand and the intention is to make the main thread wait for all tasks in the threadPool to finish, the time at which all tasks have been submitted is known (after the find-and-submit -task method has scanned through all the files).

I can simply call ThreadPoolExecutor.shutdown() and awaitTermination(Long.MAX_VALUE) so that the main thread will wait indefinitely for the ThreadPool to finish its tasks.

TommyQ
  • 501
  • 7
  • 18