Object lock in Java

Question

I am trying to understand "synchronized block" in Java. I have written very basic code to see what happens if I lock and change the object in thread_1 and access to it from another thread_2 (race condition) via another method. But I am in trouble to understand the behaviour because I was expecting Thread_1 would change value first and then Thread_2 would access the new value but the result was not as I expected.

public class Example {

public static void main(String[] args){

  final Counter counter = new Counter();

  Thread  threadA = new Thread(new Runnable() {

     @Override
     public void run() {
         System.out.println("THREAD_1_START");
         counter.add(1);
         System.out.println("THREAD_1_END");
     }
  });
  Thread  threadB = new Thread(new Runnable() {

     @Override
     public void run() {
         System.out.println("THREAD_2_START");
         System.out.println("GET_A_BY_THREAD_2:"+counter.getA(2));
         System.out.println("THREAD_2_END");

     }
 });
  threadA.start();
  threadB.start();
 }
}

public class Counter{
String A = "NONE";

public void add(long value){
    synchronized (A) {
        System.out.println("LOCKED_BY_"+value);

        for(int i = 0; i < 1000000000; i++ ){}

        setA("THREAD_"+value);
        System.out.println("GET_A_BY_THREAD:"+getA(value));
    }
}

public void setA(String A){
        System.out.println("Counter.setA()");
        this.A = A;
        System.out.println("Counter.setA()_end");
}

public String getA(long value){
    System.out.println("Counter.getA()_BY_"+value);
    return this.A;
}
}

The Output is:

THREAD_1_START
THREAD_2_START
LOCKED_BY_1
Counter.getA()_BY_2
GET_A_BY_THREAD_2:NONE
THREAD_2_END
Counter.setA()
Counter.setA()_end
Counter.getA()_BY_1
GET_A_BY_THREAD:THREAD_1
THREAD_1_END

Thread_1 locks the "A" string object and changes it but Thread_2 can read the value before it changes. When "A" is in lock, how can thread_2 can access the "A" object?

In addition to the already given answers, you can mark `String a` as `final`, to make sure the synchronized object remains the same. — MC Emperor, Sep 06 '16 at 11:50

Marko Topolnik · Accepted Answer · 2016-09-06T12:03:19.337

4

Thread_1 locks the "A" string object and changes it

No, Thread_1 locks on the "NONE" string object, creates a new String object, and overwrites the A field with a reference to this new object. Thread_2 can now acquire its free lock, however in your current code the getA method doesn't even attempt to acquire it.

You must use locking for all access, not just writing. Therefore getA must also contain a synchronized block.

The general rule is to never use mutable instance fields for locking. Doing that provides you with no useful guarantees. Therefore your A field should be final and you must delete all code that disagrees with that.

edited Sep 06 '16 at 12:03

answered Sep 06 '16 at 11:44

Marko Topolnik

195,646
29
319
436

What about if I remove " setA("THREAD_"+value); " line form the code? – stackalreadyoverflowed Sep 06 '16 at 11:56
Maybe the question was edited later. But Thread_1 only sets A once. After a delay caused by the empty for loop. – Jorn Vernee Sep 06 '16 at 11:57
@JornVernee You are right, I actually added that sentence to my answer later on, I undid it now. The loop probably takes a very short time because as soon as it's JIT-compiled, the whole loop is gone as dead code. – Marko Topolnik Sep 06 '16 at 11:59
@stackalreadyoverflowed Just make `A` final and delete any code that disagrees with that. Then you should have proper locking. – Marko Topolnik Sep 06 '16 at 12:00
Actually, you can check for that. But it doesn't look to me like Thread_2 tries to acquire any locks. – Jorn Vernee Sep 06 '16 at 12:00
When I set "A" as final and remove the setter and setA, result almost the same. – stackalreadyoverflowed Sep 06 '16 at 12:01
Did you add locking to `getA`? There is no mutual exclusion if only one thread wants the lock. – Marko Topolnik Sep 06 '16 at 12:02
Why do I need a lock on getA ? Does not "synchronized (A)" mean that A is in lock? – stackalreadyoverflowed Sep 06 '16 at 12:02
To get mutual exclusion and visibility guarantees. Isn't that what you're looking for? A single thread acquiring a lock is the same as no threads acquiring it. – Marko Topolnik Sep 06 '16 at 12:03
@MarkoTopolnik Does not "synchronized (A)" mean that A is in lock? – stackalreadyoverflowed Sep 06 '16 at 12:08
indeed that's what it means. You don't do it from `getA()`. – Marko Topolnik Sep 06 '16 at 12:09
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/122742/discussion-between-stackalreadyoverflowed-and-marko-topolnik). – stackalreadyoverflowed Sep 06 '16 at 12:09

Jens Schauder · Answer 2 · 2016-09-06T11:58:04.457

There are multiple things wrong with your code and your interpretation of it.

The synchronize block synchronizes only blocks using the same monitor (A in your case).

Since your getter is not synchronized it is not affected by the lock. It doesn't get blocked, might run during the first thread is holding the lock and even when it executes after the first thread released it's lock it might not see changes done by the first thread.

Add synchronization to the getter to fix this issue.
You lock on A i.e. on "NONE", but then change A to point to a different String. The next String comming along sees (actually might see) the new reference, and locks on the new String so again you have no two synchronized blocks that lock on the same object.

You almost always want to lock on the instance that contains the synchronized block or on it's class. This is exactly what you get when you use synchronized (static) methods instead of synchronized blocks.
Your wording is of, and it is important:

You lock on the object A and change the instance of Counter.

You cannot lock an object and you cannot change a String. You can only change References to point to a new String.

Object lock in Java

2 Answers2