Threads with shared integer object not working as expected

Question

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

score 1 · Answer 1 · answered Feb 18 '19 at 18:59

Java Integer cannot be passed by reference. On your code, each thread will create a copy of the variable. However atomicInteger can be passed by reference.

Also, to get the correct result, you can change the num variable to static variable.

public static Integer num = 1;

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

score 0 · Answer 2 · answered Feb 18 '19 at 18:58

0

Your problem is that the Integer class is Immutable, so you cannot use it in separate threads to reference a shared value. Answer: Create your own, Mutable, Integer class.

You can find a similar question answered on SO here

answered Feb 18 '19 at 18:58

Design.Garden

3,607
25
21

score 0 · Answer 3 · answered Feb 18 '19 at 19:12

Just for your knowledge, instead of using a synchronized block, on an Object, you might want to experiment with Lock(s) (e.g. ReentrantLock) and their associated Condition(s).

Using Condition(s) you can manage your shared resources in a mutually exclusive way between threads.

Ravindra Ranwala · Accepted Answer · 2019-02-19T03:54:34.337

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

I had missed the static field in my integer variable after doing this it works as expected. Can you tell me why client provided lockes shouldn't be used for synchronization. What is the alternative if instead of single thread class we have two different threads. — agnihp, Feb 19 '19 at 05:42
Your class should encapsulate synchronization policy. If you ask your client to pass in the lock object, your client knows you are doing synchronization using that lock. This violates encapsulation of your synchronization policy. Moreover you can't trust your client. If the same lock is used else where, a lock contention may occur. However I don't understand your second question. For more details read this book: https://www.amazon.com/gp/product/0321349601/ref=as_li_tl?ie=UTF8&camp=1789&creative=9325&creativeASIN=0321349601&linkCode=as2&tag=briangoetz-20&linkId=d1410b098c65f3549c71f5023b34b513 — Ravindra Ranwala, Feb 19 '19 at 05:46

Threads with shared integer object not working as expected

4 Answers4