1

When I want to update with either synchronized keyword or lock, it works when the sum variable is int but not when it is an Integer object.

code looks like this -

public class TestSynchronized {
  private  Integer sum = new Integer(0);

  public static void main(String[] args) {
    TestSynchronized test = new TestSynchronized();
    System.out.println("The sum is :" + test.sum);
  }

  public TestSynchronized() {
    ExecutorService executor = Executors.newFixedThreadPool(1000);

    for (int i = 0; i <=2000; i++) {
      executor.execute(new SumTask());
    }

    executor.shutdown();

    while(!executor.isTerminated()) {
    }
  }

  class SumTask implements Runnable {
    Lock lock = new ReentrantLock();

    public void run() {
    lock.lock();

      int value = sum.intValue() + 1;
      sum = new Integer(value);

        lock.unlock(); // Release the lock
      }

  }
}
DazedNConfused
  • 189
  • 2
  • 13
  • 3
    `Lock lock = new ReentrantLock();` you are creating a new Object for every thread – Scary Wombat Sep 20 '18 at 05:32
  • For this use case, I'd suggest using `AtomicInteger` and `getAndIncrement` or `incrementAndGet`. Then you don't need the locks any more. – jokster Sep 20 '18 at 06:22
  • Your incrementing of `sum` would be far more easily (and equivalently) written as `sum++;` (except that you'd then be using cached Integer instances sometimes). Autoboxing does what you are doing here "for free". – Andy Turner Sep 20 '18 at 07:25

1 Answers1

2

The problem is that you've separate locks for each of the SumTask objects. These objects should share the locks instead.

Create a lock object once in TestSynchronized() method. This should be shared by all the new SumTask(lock) objects.

So your SumTask class would look like:

class SumTask implements Runnable {
    Lock lock;

    public SumTask(Lock commonLock) {
        this.lock = commonLock;
    }

    public void run() {
        lock.lock();

        int value = sum.intValue() + 1;
        sum = new Integer(value);

        lock.unlock(); // Release the lock
    }

}

And don't forget to create a commonLock object in TestSynchronized() method:

  Lock lock = new ReentrantLock();

  executor.execute(new SumTask(commonLock));
Shanu Gupta
  • 3,699
  • 2
  • 19
  • 29
  • OP said "it works when the sum variable is int" I think it is worth pointing out that this is, at best, mere fluke. – Andy Turner Sep 20 '18 at 07:29