CODEWITHSUNDEEP
javathread-safety
smatter
smatter

Reputation: 29168

Thread safety in Java class

Why is this java class not Thread safe.

class TestClass {  
   private int x;

   int get() {
       return x;
   }

   void set(int x) {
       this.x = x;
   }  
}

I read that keyword synchronized is needed to make it thread safe? After all isn't the operations done inside atomic?

Upvotes: 18

Views: 1149

Answers (4)

fastcodejava
fastcodejava

Reputation: 41077

When you have two method modifying/accessing a non-volatile variable it is never thread safe. If you want to have just one method you can try :

synchronized int getAndSet(int x, boolean set) {
    if (set) this.x = x;
    return this.x;   // param x is for set
}

Upvotes: 0

Alexander Pogrebnyak
Alexander Pogrebnyak

Reputation: 45576

With multiple processors, some values may be cached by the processor and may not reflect the changes made by other threads/processors for the same objects. Actually, JVM may be implemented to work this way even with a single processor.

Synchronized methods are explicitly required by language specification to present a memory barrier and require reread of all instance variables from the memory.

Because your code is not synchronized, one thread may set the value, but the other thread will return the value still cached by that thread.

Please read 'Memory and Locks' chapter of Java Language Specification.

Upvotes: 9

Eyal Schneider
Eyal Schneider

Reputation: 22446

Although the assignment itself is an atomic operation, due to different hardware and compiler implementations, different threads may see different values of the member x. I.e., a modification by one thread may be invisible to the other thread, because of some kind of caching. This is usually called a thread visibility problem.

You can synchronize your code properly either by synchronizing on a monitor (using the synchronized keyword or the java.util.concurrent locks), or by declaring x to be volatile.

Upvotes: 14

Michael Barker
Michael Barker

Reputation: 14378

Because the field 'x' is not declared volatile there is no requirement for the JVM to ensure that 'x' is visible to all other threads. I.e. if one thread is constantly reading the value of 'x' and another thread is writing it, it is possible that the reading thread will never "see" the value change.

A synchronized keyword is not required, but will work as it will create the necessary memory barrier/cache flush to ensure 'x' is visible, but using the volatile keyword in this case will be more efficient.

Upvotes: 6

Related Questions