synchronise on non-final object [duplicate]

Question

private volatile Object obj = new MyObject();
void foo() 
{
  synchronized(obj)
  {
    obj.doWork();
  }
}
void bar()
{
  synchronized(obj)
  {
    obj.doWork();
    obj = new MyObject(); // <<<< notice this line (call it line-x)
  }
}

Suppose at a certain point in time, a thread t_bar is executing bar(), and another one t_foo is executing foo, and that t_bar has just acquired obj, so t_foo is, in effect, waiting.

After the sync-block in bar is executed, foo will get to execute its sync-block, right? What value of obj would it see? The old one? Or the new one set in bar?

(I would hope that the new value is seen, that's the whole point of coding it that way, but I want to know if this is a 'safe' bet)

Answer 1

In the exact situation you described, yes, the read of obj inside foo's synchronized block will see the new value set by the previous bar's synchronized block.

The fun part is, it doesn't always happen in that exact situation. The program is not thread safe, for example, if immediately after bar() exits, the same threads invokes another bar(), while the foo thread is locking on the old object. The bar thread locks on the new object, so the two threads are executing concurrently, both executing obj.doWork() on the same new obj.

We can probably partially fix it by

// suppose this line happens-before foo()/bar() calls
MyObject obj = new MyObject();

void foo()
    while(true)
        MyObject tmp1 = obj;
        synchronized(tmp1)
            MyObject tmp2 = obj;
            if(tmp2==tmp1)
                tmp2.doWork();
                return;
            // else retry

this at least guarantees no current invocations of obj.doWork() on the same obj, since obj.doWork() can only occur in a synchronized block that locks the exact same obj

Answer 2

It will behave normally as if object reference was not changed internally. Reason being that test for lock on object will be done only once. So even if object changes internally, thread will keep waiting and behaviour will remian same as if object was same [unchanged].

I tried another thing. I placed a sleep statement just after new object was created and then started the next thread and as expected both threads started working simultaneously. See the code below.

public class ChangeLockObjectState {

    private volatile Object obj = new Object();

    void foo() {
        synchronized (obj) {
            try {
                System.out.println("inside foo");
                Thread.sleep(10000);
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }
        }
    }

    void bar() {
        synchronized (obj) {
            try {
                System.out.println("inside bar");
                Thread.sleep(5000);
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }

            obj = new Object(); // <<<< notice this line (call it line-x)

            System.out.println("going out of  bar");

            try {

                Thread.sleep(5000);
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }

            System.out.println("wait over");

        }
    }

    /**
     * @param args
     * @throws InterruptedException
     */
    public static void main(String[] args) throws InterruptedException {
        final ChangeLockObjectState test = new ChangeLockObjectState();

        new Thread(new Runnable() {

            @Override
            public void run() {
                test.bar();

            }
        }).start();

        Thread.sleep(6000);

        new Thread(new Runnable() {

            @Override
            public void run() {
                test.foo();

            }
        }).start();

    }

}