Why is getter method required to be synchronized for the class to be Thread Safe

1) It is not. Only the set method needs to be synchronised.
2) It specifies the the field "value" should only be accessed when a specified lock is held (in your case "this", i.e. the containing object (the object of which the field "value" is a member)).

Hi,
Here is a complete snapshot of the code.
This is from the book  Java concurrency in practice.
It mentions that getter should be synchronized.


I guess its something like this but not sure.
Since if get is not synchronized one thread could be in set method and one thread can be in get method at the same time.
consider a case -
Thread 1 - inside set
Thread 2 - inside get
thread 3 - going to call get

Suppose Thread 2 gets executed first it will have old value after that set gets executed.
now value of Thread3 will depend on the order of execution of set and get method.
ITs not consistent. it could be new value or even old value.
So if two different threads call get at the same time they may get entirely different result
Or a thread could see a stale value even though set has been executed completely.

This is what i think...

But i still have some confusion. As he mentions that there is a possibility of seeing stale values.
There are two things here
1) The Java Memory Model which caches values and so the latest value is not visible to other threads.

2) The stale value happens because of two threads being simultaneously inside set and get .
eg - Set sets 100 as a value but get returns the old value because it already fetched it from inside the get.

I dont know which stale value is the book talking about here.

Yes this could happen. The safest way is to define an AtomicInteger or synchronise both methods, or use the GuardedBy (I think this actually synchronises both methods)..

Also there is one strange thing.
I wrote one class :

import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Executor;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

public class Test implements  Runnable{
    private static SynchronizedInteger synchronizedInteger = new SynchronizedInteger();

    public static void main(String[] args) {
        ExecutorService executor = Executors.newFixedThreadPool(5);
        Runnable runnable = new Test();
        executor.execute(runnable);
        System.out.println("After runnable");
        synchronizedInteger.set(100);
        System.out.println("After set 100");
    }


    @Override
    public void run() {
        while(synchronizedInteger.get()!=100) {
            System.out.println("not 100");
        }
        System.out.println("its 100");
    }
}

Select allOpen in new window

I modified the class to following:

public class SynchronizedInteger {
   private int value;

  public  int get() {
    return value;
  }
  public  void set(int value) {
    this.value = value;
  }

}

Select allOpen in new window

ie. removed all synchronization.

But still i see in the output proper values.
I mean for stale values to happen i should see something like
After set 100
not 100
not 100
not 100
.
.
.
then at some point it may get the latest value and prints
its 100

But i never see such a case at least i tried running the code 20 times in Intellij Idea but it doesnt happen.

so even if i made all the values synchronized or not practically i dont get stale values at all may be i am getting lucky or there is some reason behind it.

But i never see such a case at least i tried running the code 20 times in Intellij Idea but it doesnt happen.

This is because you only have one thread. Try it with the following:

public static void main(String[] args) {
        ExecutorService executor = Executors.newFixedThreadPool(5);
        Runnable runnable = new Test();
Runnable runnable2 = new Test();
Runnable runnable3 = new Test();
        executor.execute(runnable);
        executor.execute(runnable2);
        executor.execute(runnable3);
        System.out.println("After runnable");
        synchronizedInteger.set(100);
        System.out.println("After set 100");
    }

Select allOpen in new window

@GuardedBy defines the lock object:

• this : The intrinsic lock of the object in whose class the field is defined.

• class-name.this : For inner classes, it may be necessary to disambiguate 'this'; the class-name.this designation allows you to specify which 'this' reference is intended
• itself : For reference fields only; the object to which the field refers.
• field-name : The lock object is referenced by the (instance or static) field specified by field-name.
• class-name.field-name : The lock object is reference by the static field specified by class-name.field-name.
• method-name() : The lock object is returned by calling the named nil-ary method.
• class-name.class : The Class object for the specified class should be used as the lock object.

And to clarify @GuardedBy, it is optional and it does not have any effect on the code

I didn't know this, thanks for clarifying it.