race condition in OrderedThreadPoolExecutor/UnorderedThreadPoolExecutor

[slump]

I think I found a race condition in 
OrderedThreadPoolExecutor/UnorderedThreadPoolExecutor

I write a textline echo server using MINA

I add a UnorderedThreadPoolExecutor in server's filter:

 acceptor.getFilterChain().addLast("executor",
            new ExecutorFilter(new UnorderedThreadPoolExecutor(10)));

 and in messageReceived, I add sleep(3000) before echo the line received

when I send 10 lines from client , I think the server should use 10 threads to 
process 10 message concurrently
but the server only use 2 thread or 4 thread , and the remain message must wait 
until sleep complete

I changed executor to  ThreadPoolExecutor in JDK, it will use 10 thread

I checked and trace the code of  UnorderedThreadPoolExecutor, find the reason:
   private void addWorkerIfNecessary() {
        if (idleWorkers.get() == 0) {
            synchronized (workers) {
                if (workers.isEmpty() || idleWorkers.get() == 0) {
                    addWorker();
                }
            }
        }
    }

there may be some race condition:
1 thread A call executor's execute to sunmit one task
2 executor find there are no idle worker, so create a new thread and start it, 
now idleWorkers is 1 
3 before new worker thread fetch task and decrement the idleWorkers, the thread 
A submit more task
4 the executor find idleWorkers is 1 , so don't create new thread
5 the new worker thread fetch task and decrement the idleWorkers, now 
idleWorkers is 0 ,but there are some task left in queue without worker

MAYBE the   idleWorkers.get() == 0 should be change to idleWorkers.get() < 
getQueue().size() ?

And in OrderedThreadPoolExecutor  , have the same race condition.


And I have one more question: 
Why not just use  ThreadPoolExecutor in JDK instead of  
UnorderedThreadPoolExecutor? 
does UnorderedThreadPoolExecutor have some special requirement which  
ThreadPoolExecutor can't fulfil?
       



dingli
2008-09-04