Volatile keyword without static not working as expected

Question

I understood the difference between volatile and static keywords on variables.

static variables can be changed by different instances whereas Volatile variables can be changed by different threads.

But the below program(Copied from internet and little modified) hangs, if i remove the static keyword for MY_INT variable.

The update to variable MY_INT should be seen by other thread even without static key word. But if i remove static it hangs.

Please help me to understand this issue.

public class PrintOddAndEven extends Thread {
    static volatile  int i = 1;

    Object lock;

    PrintOddAndEven(Object lock) {
        this.lock = lock;
    }

    public static void main(String ar[]) {
        Object obj = new Object();
        PrintOddAndEven odd = new PrintOddAndEven(obj);
        PrintOddAndEven even = new PrintOddAndEven(obj);
        odd.setName("Odd");
        even.setName("Even");
        odd.start();
        even.start();
    }

    @Override
    public void run() {
        while (i <= 10) {
            if (i % 2 == 0 && Thread.currentThread().getName().equals("Even")) {
                synchronized (lock) {
                    System.out.println(Thread.currentThread().getName() + " - " + i);
                    i++;
                    lock.notify();
                }
            }
            if (i % 2 == 1 && Thread.currentThread().getName().equals("Odd")) {
                synchronized (lock) {
                    System.out.println(Thread.currentThread().getName() + " - " + i);
                    i++;

                    try {
                        lock.wait();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                }
            }
        }
    }
}

Answer 1

Your bug is due to the fact that if you remove the static keyword from the field i, you will have one different field i per instance of PrintOddAndEven so here as you have 2 instances, you have 2 different fields i such that the Even thread will loop forever as its i is never modified and the Odd thread waits forever for the same reason. When you declare the field as static, the threads share the same field i such that you don't face your bug.

You should create a dedicated class that will hold your counter and use an instance of it as object monitor that you will share between the PrintOddAndEven instances, as next:

public class MyClass {
    volatile int i = 1;
}

public class PrintOddAndEven extends Thread {

    MyClass lock;

    PrintOddAndEven(MyClass lock) {
        this.lock = lock;
    }

    public static void main(String[] args) throws Exception {
        MyClass obj = new MyClass();
        PrintOddAndEven odd = new PrintOddAndEven(obj);
        PrintOddAndEven even = new PrintOddAndEven(obj);
        odd.setName("Odd");
        even.setName("Even");
        odd.start();
        even.start();
    }

    @Override
    public void run() {
        while (lock.i <= 10) {
            if (lock.i % 2 == 0 && Thread.currentThread().getName().equals("Even")) {
                synchronized (lock) {
                    System.out.println(Thread.currentThread().getName() + " - " + lock.i);
                    lock.i++;
                    lock.notify();
                }
            }
            if (lock.i % 2 == 1 && Thread.currentThread().getName().equals("Odd")) {
                synchronized (lock) {
                    System.out.println(Thread.currentThread().getName() + " - " + lock.i);
                    lock.i++;

                    try {
                        lock.wait();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                }
            }
        }
    }
}

---

If you have only a counter, you could also consider using an instance of class AtomicInteger as counter and object monitor. The code will be the same as above except that you will create an instance using new AtomicInteger(1) to initialize the counter to 1 and then use get() to get the current value and incrementAndGet() to increment the counter.