Java: ExecutorService thread synchronization with CountDownLatch causes dead lock?

Question

I have written a game of life for programming practice. There are 3 different implementations of the generator. First: One main thread + N sub threads, Second: SwingWorker + N sub threads, Third: SwingWorker + ExecutorService. N is the number of availableProcessors or user defined. The first two implementations runs fine, with one and more threads. The implementation with the ExecutorServise runs fine with one thread, but locks with more than one. I tried everything, but i can't get the solution.

Here the code of the fine workling implementation (second one):

    package example.generator;

    import javax.swing.SwingWorker;

    /**
     * AbstractGenerator implementation 2: SwingWorker + sub threads.
     * 
     * @author Dima
     */
    public final class WorldGenerator2 extends AbstractGenerator {


        /**
         * Constructor.
         * @param gamePanel The game panel
         */
        public WorldGenerator2() {
            super();
        }




        /* (non-Javadoc)
         * @see main.generator.AbstractGenerator#startGenerationProcess()
         */
        @Override
        protected void startGenerationProcess() {
            final SwingWorker<Void, Void> worker = this.createWorker();
            worker.execute();
        }




        /**
         * Creates a swing worker for the generation process.
         * @return The swing worker
         */
        private SwingWorker<Void, Void> createWorker() {
            return new SwingWorker<Void, Void>() {

                @Override
                protected Void doInBackground() throws InterruptedException {
                    WorldGenerator2.this.generationProcessing();
                    return null;
                }
            };
        }





        /* (non-Javadoc)
         * @see main.generator.AbstractGenerator#startFirstStep()
         */
        @Override
        public void startFirstStep() throws InterruptedException {
            this.getQueue().addAll(this.getLivingCells());
            for (int i = 0; i < this.getCoresToUse(); i++) {
                final Thread thread = new Thread() {
                    @Override
                    public void run() {
                        WorldGenerator2.this.fistStepProcessing();
                    }
                };
                thread.start();
                thread.join();
            }
        }




        /* (non-Javadoc)
         * @see main.generator.AbstractGenerator#startSecondStep()
         */
        @Override
        protected void startSecondStep() throws InterruptedException {
            this.getQueue().addAll(this.getCellsToCheck());
            for (int i = 0; i < this.getCoresToUse(); i++) {
                final Thread thread = new Thread() {

                    @Override
                    public void run() {
                        WorldGenerator2.this.secondStepProcessing();
                    }
                };
                thread.start();
                thread.join();
            }
        }

    }

Here is the code of the not working implementation with executor service:

    package example.generator;

    import java.util.concurrent.CountDownLatch;
    import java.util.concurrent.ExecutorService;
    import java.util.concurrent.Executors;

    import javax.swing.SwingWorker;

    /**
     * AbstractGenerator implementation 3: SwingWorker + ExecutorService.
     * 
     * @author Dima
     */
    public final class WorldGenerator3 extends AbstractGenerator {


        private CountDownLatch  countDownLatch;
        private ExecutorService executor;


        /**
         * Constructor.
         * @param gamePanel The game panel
         */
        public WorldGenerator3() {
            super();
        }




        /* (non-Javadoc)
         * @see main.generator.AbstractGenerator#startGenerationProcess()
         */
        @Override
        protected void startGenerationProcess() {
            this.executor = Executors.newFixedThreadPool(this.getCoresToUse());
            final SwingWorker<Void, Void> worker = this.createWorker();
            worker.execute();
        }




        /**
         * Creates a swing worker for the generation process.
         * @return The swing worker
         */
        private SwingWorker<Void, Void> createWorker() {
            return new SwingWorker<Void, Void>() {

                @Override
                protected Void doInBackground() throws InterruptedException {
                    WorldGenerator3.this.generationProcessing();
                    return null;
                }
            };
        }




        /* (non-Javadoc)
         * @see main.generator.AbstractGenerator#startFirstStep()
         */
        @Override
        public void startFirstStep() throws InterruptedException {
            this.getQueue().addAll(this.getLivingCells());
            this.countDownLatch = new CountDownLatch(this.getCoresToUse());
            for (int i = 0; i < this.getCoresToUse(); i++) {    
                this.executor.execute(new Runnable() {  
                    @Override
                    public void run() {
                        WorldGenerator3.this.fistStepProcessing();
                        WorldGenerator3.this.countDownLatch.countDown();
                    }
                });
            }
            this.countDownLatch.await();

        }




        /* (non-Javadoc)
         * @see main.generator.AbstractGenerator#startSecondStep()
         */
        @Override
        protected void startSecondStep() throws InterruptedException {
            this.getQueue().addAll(this.getCellsToCheck());
            this.countDownLatch = new CountDownLatch(this.getCoresToUse());
            for (int i = 0; i < this.getCoresToUse(); i++) {
                this.executor.execute(new Runnable() {
                    @Override
                    public void run() {
                        WorldGenerator3.this.secondStepProcessing();
                        WorldGenerator3.this.countDownLatch.countDown();
                    }
                });
            }
            this.countDownLatch.await();

        }
    }

Here you can download, a sample of my application, with a small launcher. it prints only the result of a iteration on the console: Link

---

Now my code looks like this:

/* (non-Javadoc)
 * @see main.generator.AbstractGenerator#startFirstStep()
 */
@Override
public void startFirstStep() throws InterruptedException {

    this.getQueue().addAll(this.getLivingCells());      

    final ArrayList<Callable<Void>> list = new ArrayList<Callable<Void>>(this.getCoresToUse());

    for (int i = 0; i < this.getCoresToUse(); i++) {

        list.add(new Callable<Void>() {

                @Override
                public Void call() throws Exception {
                    WorldGenerator3.this.fistStepProcessing();
                    return null;
                }
            }
        );          
    }

    this.executor.invokeAll(list);
}

But here is again the same problem. If I run it with one core (thread) there are no problems. If I set the number of cores to more than one, it locks. In my first question there is a link to a example, which you can run (in eclipse). Maybe I overlook something in the previous code.

Answer 1

In all variants you are executing threads in serial rather than parallel because you join and await inside the for-loop. That means that the for-loop cannot move on to the next iteration until the thread just started is complete. This amounts to having only one thread live at any given time -- either the main thread or the one thread created in the current loop iteration. If you want to join on multiple threads, you must collect the refs to them and then, outside the loop where you started them all, enter another loop where you join on each one.

As for using CountDownLatch in the Executors variant, what was said for threads goes for the latch here: don't use an instance var; use a local list that collects all latches and await them in a separate loop.

But, you shouldn't really be using the CountDownLatch in the first place: you should put all your parallel tasks in a list of Callables and call ExecutorService.invokeAll with it. It will automatically block until all the tasks are done.

Answer 2

I find your usage of Executors facilities a little bit odd...

I.e. the idea is to have Executor with a pool of threads, size of which usually is related to number of cores your CPU supports. Then you submit whatever number of parallel tasks to the Executor, letting it to decide what to execute when and on which available Thread from its pool.

As for the CountDownLatch... Why not use ExecutorService.invokeAll? This method will block untill all submitted tasks are completed or timeout is reached. So it will do counting of the work left on your behalf. Or a CompletionService which "decouples the production of new asynchronous tasks from the consumption of the results of completed tasks" if you want to consume Task result as soon as it becomes available i.e. not wait for all tasks to complete first.

Something like

    private static final int WORKER_THREAD_COUNT_DEFAULT = Runtime.getRuntime().availableProcessors() * 2;

    ExecutorService executor = Executors.newFixedThreadPool(WORKER_THREAD_COUNT);

    // your tasks may or may not return result so consuming invokeAll return value may not be necessary in your case 
    List<Future<T>> futuresResult = executor.invokeAll(tasksToRunInParallel, EXECUTE_TIMEOUT,
                TimeUnit.SECONDS);