Java synchronization(object locking) not working as expected

Question

Please consider the below code.

import static java.lang.System.out;

public class Task
{
    public Integer k =  new Integer(10) ;

    public Task()
    {
        out.println(k + " constructor of Task : " + Thread.currentThread().getName());
    }
}

import static java.lang.System.out;

public class Executor2 implements Runnable
{
    private Task task;
    public Executor2(Task t)
    {
        out.println("constructor of Executor2 : " + Thread.currentThread().getName());
        task = t;
    }

    @Override
    public void run()
    {
        synchronized(task.k)
        {
            task.k = 88;
            out.println("changed value of task.k to : " + task.k + " " + Thread.currentThread().getName());
            try
            {
                out.println("sleeping");
                Thread.sleep(5000);
            }
            catch (InterruptedException ex)
            {
                ex.printStackTrace(out);
            }
            out.println("done");
        }
    }
}

import static java.lang.System.out;

public class Executor3 implements Runnable
{
    private Task task;
    public Executor3(Task t)
    {
        out.println("constructor of Executor3 : " + Thread.currentThread().getName());
        task = t;
    }

    @Override
    public void run()
    {
        synchronized(task.k)
        {
          task.k = 888;
          out.println("changed value of task.k to : " + task.k + " " + Thread.currentThread().getName());
        }
    }
}
------------------------------------------------------------
public class Main
{
    public static void main(String[] args)
    {
        Task task = new Task();

        Executor2 executor2 = new Executor2(task);
        Thread thread2 = new Thread(executor2);
        thread2.start();

        Executor3 executor3 = new Executor3(task);
        Thread thread3 = new Thread(executor3);
        thread3.start();
    }
}

Below is the output of the program.

10 constructor of Task : main
constructor of Executor2 : main
constructor of Executor3 : main
changed value of task.k to : 88 Thread-0
sleeping
changed value of task.k to : 888 Thread-1
done

The surprising thing here is output line : changed value of task.k to : 888 Thread-1 which is not expected to be printed before output line : done. How come the lock on Integer object is released before sleep duration has passed?

Thanks.

possible duplicate of stackoverflow.com/questions/16280699/… — Radiodef
– Radiodef, Commented May 21, 2015 at 20:36
Bear in mind that even though you started thread2 before thread3, it is possible for thread3's run method to be invoked first. They might or might not attempt to synchronize on the same object, since each changes task.k. — VGR
– VGR, Commented May 21, 2015 at 20:57

David Schwartz · Accepted Answer · 2015-05-21 21:38:04Z

2

    synchronized(task.k)
    {
      task.k = 888;

Changing the object you're synchronizing on kind of defeats the point of the synchronization. You then try to print the new object while you're still synchronized on the old one. Don't replace objects threads are synchronizing on!

answered May 21, 2015 at 21:38

David Schwartz

184k18 gold badges229 silver badges292 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

The Roy Over a year ago

Interesting. Isn't the synchronised lock implemented Using the object's monitor lock which in turn is used for protecting exclusive access to the object's state. So if the state is changed after holding the lock, why is there an issue?

David Schwartz Over a year ago

@DROY It's not the object's state that's changed, the object is replaced with a whole new object. Remember, Integer is immutable, so if it has a different value, it's a different object.

The Roy Over a year ago

Got it. I overlooked the specific implementation used here. But if I had used a mutable data member for lock and changed the state, it should have been ok?

David Schwartz Over a year ago

@DROY Yes. You got it.

Solomon Slow · Accepted Answer · 2015-05-21 21:45:13Z

1

Extending what David Schwartz said: In almost all cases, if you're going to write this:

synchronized (foo) {
    ...
}

Then the variable that refers to the lock object should be a final field:

class MyClass {

    final Object foo = new Object();
    ...
    void someMethod(...) {
        synchronized (foo) {
            ...
        }
    }
    ...
}

That will prevent you from making the same mistake in the future.

answered May 21, 2015 at 21:45

Solomon Slow

27.6k5 gold badges39 silver badges61 bronze badges

Comments

user1861417 · Accepted Answer · 2016-03-06 01:45:23Z

0

Agree with above. Also while using private lock, one pitfall is not to lock on string literals - String literals are shared resource.

private final String lock = “xx”;

private final String lock = new String(“xxx”);

The second locking is fine.

answered Mar 6, 2016 at 1:45

user1861417

295 bronze badges

Collectives™ on Stack Overflow

Java synchronization(object locking) not working as expected

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related