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?

score 1 · Accepted Answer · answered Jun 16 '13 at 17:12

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);

Thanks a lot. I wanted to create boolean done variable in Runnable class but mistakenly create in Solver class. If I create it inside Worker class its working fine. Thanks a lot. — Sohan Badaya, Jun 16 '13 at 17:38

CyclicBarrier code not working?

1 Answers1