CODEWITHSUNDEEP
javathread-safetysynchronized
SubMachine
SubMachine

Reputation: 507

Invoking synchronized getter and setter

I am trying to practice synchronize keyword with methods.

I wrote the following code:

Adder class:

public class Adder implements Runnable{
    Counter counter;
    Adder(Counter counter){
        this.counter = counter;
    }
    public void run() {
        for (int i=0; i<100; i++)
            counter.setCount(counter.getCount()+1);
    }
}

Counter class:

public class Counter {

    private int count = 0;

    public synchronized void setCount(int val){
        count = val;
    }

    public synchronized int getCount(){
        return count;
    }
}

main:

public class main {

    public static void main(String[] args) {
        Counter counter = new Counter();
        Adder adder = new Adder(counter);
        Thread t1 = new Thread(adder);
        Thread t2 = new Thread(adder);
        t1.start();
        t2.start();
        try {
            t1.join();
            t2.join();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        System.out.println(counter.getCount());
    }
}

I would expect the output of this to be 200, but it's not deterministic (theoretically, can have any value between 0-200). I suspect the problems is that I am using the getter and setter inline, i.e.

counter.setCount(counter.getCount()+1);

For some reason this "breaks" the mutual exclusion that I am trying to achieve with synchronization, but I can't see why.

I implemented the 1's addition with count++ like so:

public synchronized void add1(){
        count++;
}

This worked, maybe because this way I use only one function instead of two inline. Could you explain why the first implementation doesn't work?

Upvotes: 2

Views: 1144

Answers (2)

lexicore
lexicore

Reputation: 43689

Calling the getter and subsequent calling of setter is two independent operations. "Set the result of getter plus one" is not atomic here. So you may perfectly have two gets returning the same value, and two sets of the same value increased by one.

Assume count is 100. You have two threads calls calling the getter, both getting 100. Then they both call the setter, setting 101. So the counter is now 101, not 102 - and both threads "were there" already.

So the result is non-deterministic and depends on the actual order of get/set operations from the two threads.

Upvotes: 1

Vasu
Vasu

Reputation: 22442

counter.setCount(counter.getCount()+1); is NOT atomic and it involves 3 steps:

(1) Read the value of count

(2) Add one to count

(3) Write the value of count

In the first approach, you are getting the locks independently i.e., in between the get and set calls of one thread there will be an interference of other threads. So you can't guarantee that the first thread read value is the same as when it comes for the writing.

In the second approach, you are holding the lock and performing all of the above 3 steps, so you will not find any problem.

Also, you can also solve your problem by using the threadsafe AtomicInteger class.

Upvotes: 1

Related Questions