1

I am attempting to reimplement my concurrent code using CyclicBarrier which is new to me. I can do without it but am time trialling it against my other solution, the problem I have is a deadlock situation with the following code:

//instance variables (fully initialised elsewhere).
private final ExecutorService exec = Executors.newFixedThreadPool(4);
private ArrayList<IListener> listeners = new ArrayList<IListener>();
private int[] playerIds;

private class WorldUpdater {
    final CyclicBarrier barrier1;
    final CyclicBarrier barrier2;
    volatile boolean anyChange;
    List<Callable<Void>> calls = new ArrayList<Callable<Void>>();

    class SyncedCallable implements Callable<Void> {
        final IListener listener;

        private SyncedCallable(IListener listener) {
            this.listener = listener;
        }

        @Override
        public Void call() throws Exception {
            listener.startUpdate();
            if (barrier1.await() == 0) {
                anyChange = processCommons();
            }
            barrier2.await();
            listener.endUpdate(anyChange);
            return null;
        }
    }

    public WorldUpdater(ArrayList<IListener> listeners, int[] playerIds) {
        barrier2 = new CyclicBarrier(listeners.size());
        barrier1 = new CyclicBarrier(listeners.size());
        for (int i : playerIds)
            calls.add(new SyncedCallable(listeners.get(i)));
    }

    void start(){
        try {
            exec.invokeAll(calls);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
}


void someMethodCalledEveryFrame() {
    //Calls some Fisher-something method that shuffles int[]
    shufflePIDs();  
    WorldUpdater updater = new WorldUpdater(listeners, playerIds);
    updater.start();
}

I use the debugger in Android Studio (intelliJ) to pause execution at this stage. I get multiple threads showing the my await calls as the last of my code to be executed

->Unsafe.park

->LockSupport.park

->AbstractQueuedSynchronizer$ConditionObject.await

->CyclicBarrier.doWait

->CyclicBarrier.await

At least one thread will be have this stack:

->Unsafe.park.

->LockSupport.park

->AbstractQueuedSynchronizer$ConditionObject.await

->LinkedBlockingQueue.take

->ThreadPoolExecutor.getTask

->ThreadPoolExecutor.runWorker

->ThreadPoolExecutor$Worker.run

->Thread.run

I notice that the CyclicBarrier plays no part in these latter stray threads.

processCommons is calling exec.invokeAll (on the 3 listeners), I suppose this means I am running out of threads. But many times this doesn't happen so please could someone clarify why ExecutorService cannot consistently schedule my threads? They have their own stack and program counter so I would have thought this to not be a problem. I only ever have max 4 running at once. Someone help me with the math?

Community
  • 1
  • 1
John
  • 6,433
  • 7
  • 47
  • 82

2 Answers2

1

What is the value of listeners.size() when your WorldUpdater is created? If it is more than four, then your threads will never get past the barrier.

Your ExecutorService has exactly four threads. No more, no fewer. The callers of barrier1.await() and barrier2.await() will not get past the barrier until exactly listeners.size() threads are waiting.

My gut reaction is, it would be a mistake for pool threads to use a CyclicBarrier. CyclicBarrier is only useful when you know exactly how many threads will be using it. But, when you're using a thread pool, you often do not know the size of the pool. In fact, in a real-world (i.e., commercial) application, if you're using a thread pool, It probably was not created by your code at all. It probably was created somewhere else, and passed in to your code as an injected dependency.

Solomon Slow
  • 25,130
  • 5
  • 37
  • 57
  • I initially approved this because of your emphasis on "_exactly_ four", "No more, no fewer", which appeared profound. I now realise it is just wrong: _like_ my attempt to use `CyclicBarrier`. It just doesn't fit here, so thanks for that pointer. – John Mar 25 '16 at 08:17
0

I did a little experiment and came up with:

        @Override
        public Void call() throws Exception {
            System.out.println("startUpdate, Thread:" + Thread.currentThread());
            listener.startUpdate();
            if (barrier1.await() == 0) {
                System.out.println("processCommons, Thread:" + Thread.currentThread());
                anyChange = processCommons();
            }
            barrier2.await();
            System.out.println("endUpdate, Thread:" + Thread.currentThread());
            listener.endUpdate(anyChange);
            return null;
        }

Which revealed when using a pool of 3 with 3 listeners, I will always hang in processCommons which contains the following:

    List<Callable<Void>> calls = new ArrayList<Callable<Void>>();
    for (IListener listiner : listeners)
        calls.add(new CommonsCallable(listener));
    try {
        exec.invokeAll(calls);
    } catch (InterruptedException e) {
        e.printStackTrace();
    }

With 2 threads waiting at the barrier and the third attempting to create 3 more. I needed one extra thread in the ExecutorService and the 2 at the barrier could be "recycled" as I was asking in my question. I've got references to 6 threads at this stage when exec is only holding 4. This can run happily for many minutes.

private final ExecutorService exec = Executors.newFixedThreadPool(8);

Should be better, but it was not.

Finally I did breakpoint stepping in intelliJ (thanks ideaC!)

The problem is

        if (barrier1.await() == 0) {
            anyChange = processCommons();
        }
        barrier2.await();

Between the 2 await you may get several suspended threads that haven't actually reached the await. In the case of 3 listeners out of a pool of 4 it only takes one to get "unscheduled" (or whatever) and barrier2 will never get the full complement. But what about when I have a pool of 8? The same behaviour manifests with all but two of the threads the stack of limbo:

->Unsafe.park.

->LockSupport.park

->AbstractQueuedSynchronizer$ConditionObject.await

->LinkedBlockingQueue.take

->ThreadPoolExecutor.getTask

->ThreadPoolExecutor.runWorker

->ThreadPoolExecutor$Worker.run

->Thread.run

What can be happening here to disable all 5 threads? I should have taken James Large's advice and avoided crowbarring in this over elaborate CyclicBarrier.--UPDATE-- It can run all night now without CyclicBarrier.

John
  • 6,433
  • 7
  • 47
  • 82