Search code examples
javamultithreadingjava-threadsatomicinteger

Threads with shared integer object not working as expected


I have a problem where i have to print the numbers in such format.

First  1
First  2
Second  3
Second  4
First  5
First  6
Second  7
Second  8
First  9
and so on...

I have implemented my runnable interface as below.

class ThreadDemo implements Runnable {

 public volatile Integer num;

 public Object lock;

 public ThreadDemo(Integer num, Object lock) {
  this.num = num;
  this.lock = lock;
 }

 @Override
 public void run() {

  try {
   while (true) {
    int count = 0;
    synchronized(lock) {
     Thread.sleep(100);
     while (count < 2) {
      System.out.println(Thread.currentThread().getName() + "  " + num++);
      count++;

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

My main class is as follows

public class CoWorkingThreads {
 private static volatile Integer num = new Integer(1);
 public static void main(String...args) {
  Object lock = new Object();
  Thread thread1 = new Thread(new ThreadDemo(num, lock), "First");
  thread1.start();
  Thread thread2 = new Thread(new ThreadDemo(num, lock), "Second");
  thread2.start();

 }
}

when i run the program i am getting the output as follows

First  1
First  2
Second  1
Second  2
First  3
First  4
Second  3
Second  4

Instead of previously expected results. But when I Change the integer to atomic integer type i start getting the expected result. can anyone explain what is i can do to make it run with integer instead of using atomic integer


Solution

  • I still believe that this question is NOT answered correctly. The flaw here is that you have never marked shared data as static. So each thread has it's own copy independent of the other. Integer is an immutable wrapper class, which is true but it has nothing to do in this context. Let's dig more into num++. The ++ operator applies only to (primitive) integer types. Behind the scenes, num is unboxed, the ++ is applied, and the result is then assigned back to num (after a boxing conversion). The Integer class does not have a ++ operator. In fact, Integer objects are immutable.

    Immutable means every time you increment and create a new value object. And that new value object is assigned back to your num reference. But two threads have their own copy of num reference pointing to different Integer boxed primitives. So they increment it independently of one another which is not visible to the other. If you want to share it between threads you have to use static access modifier at the site of declaration. More over a passing two values to a shared variable does not make sense. Instead you can initialize it inline. Here's the fixed version.

    public class ThreadDemo implements Runnable {
        public static Integer num = 1;
    
        public static final Object lock = new Object();
    
        public ThreadDemo() {
        }
    
        @Override
        public void run() {
    
            try {
                while (true) {
                    int count = 0;
                    synchronized (lock) {
                        Thread.sleep(100);
                        while (count < 2) {
                            System.out.println(Thread.currentThread().getName() + "  " + num++);
                            count++;
    
                        }
                        lock.notify();
                        lock.wait();
                    }
                }
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }
    
    public class CoWorkingThreads {
        public static void main(String[] args) {
            Thread thread1 = new Thread(new ThreadDemo(), "First");
            thread1.start();
            Thread thread2 = new Thread(new ThreadDemo(), "Second");
            thread2.start();
        }
    }
    

    Finally use of a client provided lock object violates the encapsulation of synchronization policy. So I have used an internal private lock object instead.

    Here's the new output.

    First 1 First 2 Second 3 Second 4 First 5 First 6 Second 7 Second 8 First 9 First 10