CyclicBarrier code not working?

Question

I got CyclicBarrier code from oracle page to understand it more. I modified it and now having one doubt. Below code doesn't terminate but If I uncomment Thread.sleep condition, It works fine.

import java.util.Arrays;
import java.util.concurrent.BrokenBarrierException;
import java.util.concurrent.CyclicBarrier;

class Solver {
    final int N;
    final float[][] data;
    boolean done = false;
    final CyclicBarrier barrier;

    class Worker implements Runnable {
        int myRow;

        Worker(int row) {
            myRow = row;
        }

        public void run() {
            while (!done) {
                processRow(myRow);

                try {
                    barrier.await();
                } catch (InterruptedException ex) {
                    return;
                } catch (BrokenBarrierException ex) {
                    return;
                }
            }
            System.out.println("Run finish for " + Thread.currentThread().getName());
        }

        private void processRow(int row) {

            float[] rowData = data[row];

            for (int i = 0; i < rowData.length; i++) {
                rowData[i] = 1;
            }

            /*try {
                Thread.sleep(2000);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }*/
            done = true;
        }
    }

    public Solver(float[][] matrix) {
        data = matrix;
        N = matrix.length;
        barrier = new CyclicBarrier(N, new Runnable() {
            public void run() {
                for (int i = 0; i < data.length; i++) {
                    System.out.println("Data " + Arrays.toString(data[i]));
                }

                System.out.println("Completed:");
            }
        });
        for (int i = 0; i < N; ++i)
            new Thread(new Worker(i), "Thread "+ i).start();
    }
}

public class CyclicBarrierTest {
    public static void main(String[] args) {

        float[][] matrix = new float[5][5];

        Solver solver = new Solver(matrix);
    }
}

Why Thread.sleep is required in above code?

Answer 1

I've not run your code but there may be a race condition, here is a scenario that reveals it:

• you start the first thread, it runs during a certain amount of time sufficient for it to finish the processRow method call so it sets done to true and then waits on the barrier,

• the other threads start but they see that all is "done" so they don't enter the loop and they'll never wait on the barrier, and end directly

• the barrier will never be activated as only one of the N threads has reached it

• deadlock

Why it is working with the sleep:

• when one of the thread starts to sleep it lets the other threads work before marking the work as "done"

• the other threads have enough time to work and can themselves reach the barrier

• 2 seconds is largely enough for 5 threads to end a processing that should not last longer than 10ms

But note that if your system is ovrerloaded it could too deadlock:

• the first thread starts to sleep

• the OS scheduler lets another application work during more than 2 seconds

• the OS scheduler comes back to your application and the threads scheduler chooses the first thread again and lets it terminate, setting done to true

• and here again the first scenario => deadlock too

And a possible solution (sorry not tested):

change your while loops for do/while loops:

do
{
    processRow(myRow);

    ...
}
while (!done);