counter value is getting rested in multithreading

Question

I tried to print odd number in one thread and even number in another. I tried creating two thread and printing it in run method.

public class OddEven
{
    private final int MAX = 10;
    private static int counter = 0;
    private volatile boolean isOdd = true;

    public synchronized void printEven(int counter)
    {
        try {
            if (!isOdd) {
                System.out.println(Thread.currentThread().getName() + " " + counter);
                counter++;
                isOdd = true;
            }
            wait();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }

    public synchronized void printOdd(int counter)
    {
        if (isOdd) {
            counter++;
            System.out.println(Thread.currentThread().getName() + " " + counter);
            isOdd = false;
        }
        notifyAll();
    }

    public static void main(String[] args) {
        OddEven oddEven = new OddEven();

        Thread th1 = new Thread() {
            public void run() {
                while (OddEven.counter < oddEven.MAX) {
                    oddEven.printEven(OddEven.counter);
                }
            }
        };
        th1.setName("even -");
        th1.start();

        Thread th2 = new Thread() {
            public void run() {
                while (OddEven.counter < oddEven.MAX) {
                    oddEven.printOdd(OddEven.counter);
                }
            }
        };
        th2.setName("odd -");
        th2.start();
    }
}

But it is printing it like below infinitely.

even - 0
odd - 1
even - 0
odd - 1
even - 0
odd - 1

(Completely irrelevant, but `isOdd` need not be `volatile` as it is only accessed when synchronized) — Sean Bright, Jan 24 '19 at 15:37

Andrew Tobilko · Answer 1 · 2019-01-24T16:13:02.380

2

To read: Is Java "pass-by-reference" or "pass-by-value"?

You pass in a primitive. counter++; makes sense only within the method and has no impact on the outer world. count refers to the method param, not to the field this.count.
There is no proper synchronisation placed upon the condition OddEven.counter < oddEven.MAX, so different things may happen.

My advice would be to remove isOdd and do a check on the spot. For instance,

public synchronized void printEven() {
    if (counter % 2 != 0) {
        System.out.println(Thread.currentThread().getName() + " " + ++counter);
    }
}

edited Jan 24 '19 at 16:13

answered Jan 24 '19 at 15:42

Andrew Tobilko

48,120
14
91
142

He is using and modifying an instance variable of a class. This is not the issue. – Matthew Jan 24 '19 at 15:46
4

@Matthew that's the issue, a method param hides the field, so he isn't modifying the field. – Andrew Tobilko Jan 24 '19 at 15:46

score 0 · Answer 2 · answered Jan 24 '19 at 15:51

The line oddEven.printEven(OddEven.counter) passes an integer by value to the printEven method which does not change the value of OddEven.counter when it does counter++ as also pointed in other answers here.

To get the desired output, one option is to remove the passed parameter to both printEven and printOdd methods. And there are many other ways to achieve what you are trying to do here.

And there is also a mistake in the printEven method. counter++; needs to be before the print statement.

This will give you the desired output.

counter value is getting rested in multithreading

2 Answers2