Odd even printing number threads with a control thread

Question

I wrote below program in which even thread would print even numbers whereas odd thread would print odd numbers. In addition to the odd and even threads, I created a control thread, which decides if a number is odd or even and sets flag appropriately. Based on the flag which control thread sets, either odd or even thread will get chance to print.

I am using an array as source. The control thread increments the index so odd or even thread can retrieve the number from array and print.

The below is the complete code, with comments are well.

package com.example.practice;


public class OddEvenDemoVer2 {

    // Since all of these variable are used in a synchronized block, I think we
    // don't need them to be as volatile, as synchronized enforces
    // memory-barrier and hence all thread would see latest values.

    static boolean printEven = false;
    static boolean printingDone = false;
    static int index = 0;

    static volatile boolean stop = false;

    static volatile boolean oddThreadStarted = false;
    static volatile boolean evenThreadStarted = false;

    public static void main(String[] args) throws InterruptedException {

        Object _controlLock = new Object();
        Object _indexControlLock = new Object();

        int arr[] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

        class ControlThread implements Runnable {

            @Override
            public void run() {

                // Wait for proper initialization of odd and even threads.
                while (!oddThreadStarted && !evenThreadStarted) {
                    try {
                        Thread.sleep(50);
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                }

                for (; !stop;) {

                    // This is to make sure that we give chance to OddThread or
                    // EvenThread to print the values.

                    // Here, we are only setting the flag which thread should
                    // print the number, the advancing of index is done in
                    // another block.
                    synchronized (_controlLock) {

                        if (arr[index] % 2 == 0) {
                            printEven = true;
                        }
                        else {
                            printEven = false;
                        }
                        _controlLock.notifyAll();
                    }

                    // This is to make sure we advance index only when printing
                    // has been done either by OddThread or EvenThread
                    synchronized (_indexControlLock) {
                        while (printingDone != true) {
                            try {
                                _indexControlLock.wait();
                            } catch (InterruptedException e) {
                                e.printStackTrace();
                            }
                        }
                        index++;
                        if (index > 9) {
                            stop = true;
                        }
                    }
                }
            }
        }

        class EvenPrintingThread implements Runnable {

            @Override
            public void run() {
                evenThreadStarted = true;

                // Loop until stop is signaled by ControlThread
                for (; !stop;) {

                    synchronized (_controlLock) {
                        while (printEven != true) {
                            try {
                                _controlLock.wait();
                            } catch (InterruptedException e) {
                                e.printStackTrace();
                            }
                        }
                        System.out.println("Even printing thread --> " + arr[index]);

                        // This is to signal control thread that printing has
                        // been done and now index can be advanced.
                        synchronized (_indexControlLock) {
                            printingDone = true;
                            _indexControlLock.notify();
                        }
                    }
                }
            }
        }

        class OddPrintingThread implements Runnable {

            @Override
            public void run() {
                oddThreadStarted = true;

                // Loop until stop is signaled by ControlThread
                for (; !stop;) {

                    synchronized (_controlLock) {
                        while (printEven != false) {
                            try {
                                _controlLock.wait();
                            } catch (InterruptedException e) {
                                e.printStackTrace();
                            }
                        }

                        System.out.println("Odd printing thread --> " + arr[index]);

                        // This is to signal control thread that printing has
                        // been done and now index can be advanced.
                        synchronized (_indexControlLock) {
                            printingDone = true;
                            _indexControlLock.notify();
                        }
                    }
                }
            }
        }

        Thread controlThread = new Thread(new ControlThread());

        controlThread.start();

        Thread evenThread = new Thread(new EvenPrintingThread());
        Thread oddThread = new Thread(new OddPrintingThread());

        evenThread.start();
        oddThread.start();

        Thread.sleep(1000000L);
    }
}

I expected this program will work , however it is behaving erratically. For example, one of the output is:

Odd printing thread --> 1
Odd printing thread --> 1

Odd printing thread --> 1
Odd printing thread --> 1

...

Odd printing thread --> 1
Odd printing thread --> 1
Odd printing thread --> 1
Odd printing thread --> 10
Odd printing thread --> 10
Odd printing thread --> 10
Odd printing thread --> 10

I saw online some other ways in which similar problem can be solved, however, when I started on this (without looking for ready made solution online), the above approach came to my mind. I don't want to abandon simply because it isn't working. I debugged, but didn't got a clarity as to what might be wrong in this approach.

What am I missing?

EDIT

Attaching the screen shot in which shows two threads "owning" same object id.

Answer 1

Depending on the expected behavior, it only needs one or two changes.

The output that you're seeing is not wrong. Your printingDone will never be set back to false, so the controller thread will happily keep incrementing your index once it gets the chance. The notifyAll() method will wake up suspended odd/even threads, but all threads using the same lock are still competing for synchronization. My guess is that the controller thread finishes the increment fast enough to compete and therefore you have a race condition with unreliable output.

If you want at least one line for each array element, just set printingDone back to false after you incremented the index in the controller thread:

                    index++;
                    if (index > 9) {
                        stop = true;
                    }
                    printingDone = false;

If you feel like you should only get one output per value, it also makes sense to suspend your odd/even threads whenever printingDone is set to true:

                    while (printingDone == true || printEven != true) { // or printEven == true for the odd printer
                        try {
                            _controlLock.wait();
                        } catch (InterruptedException e) {
                            e.printStackTrace();
                        }
                    }

Edit: Oh, and the reason why you’re seeing even numbers printed by the odd thread is probably due to the fact that the odd thread acquires the controller lock after the index increment, but before the controller gets the chance to update printEven. Perhaps you should think of a way to both increment and update the boolean in the same code block.