synchronized depended on string initilizing

Question

I'm trying to use String object as a lock (yes, I know it's not recommended - it's only for locks-understanding).

when initializing the lock when creating it, all works well:

code:

public class Action{

    String lock  = new String("hello");

    @Override
    public String myAction(long threadId){

        synchronized (lock) {
            i++;
            printToLog("thread " + threadId);

            try {
                Thread.sleep(100000);
            } catch (InterruptedException e) {
                printToLog("Thread "+threadId+ " could not sleep "+ e.getMessage() + "\n");
            }
            printToLog("Thread "+threadId+ " slept well! ");
        }
        return i+"";
    }

    private void printToLog(String messege) {
        DateFormat dateFormat = new SimpleDateFormat("yyyy/MM/dd HH:mm:ss");
        //get current date time with Date()
        Date date = new Date(System.currentTimeMillis());
        logger.info(messege + " at time: "+dateFormat.format(date));
    }

}

logs:

[thread 555 at time: 2015/02/18 18:16:03]
[Thread 555 slept well! at time: 2015/02/18 18:17:43]
[thread 557 at time: 2015/02/18 18:17:43]
[Thread 557 slept well! at time: 2015/02/18 18:19:23]
[thread 556 at time: 2015/02/18 18:19:23]
[Thread 556 slept well! at time: 2015/02/18 18:21:03]

but when I initialize the instance member lock within the method, it's as if the synch isn't working.

code:

public class Action{

    String lock;

    @Override
    public String myAction(long threadId){
        lock = new String("hello");
        synchronized (lock) {
            i++;
            printToLog("thread " + threadId);

            try {
                Thread.sleep(100000);
            } catch (InterruptedException e) {
                printToLog("Thread "+threadId+ " could not sleep "+ e.getMessage() + "\n");
            }
            printToLog("Thread "+threadId+ " slept well! ");
        }
        return i+"";
    }

    private void printToLog(String messege) {
        DateFormat dateFormat = new SimpleDateFormat("yyyy/MM/dd HH:mm:ss");
        //get current date time with Date()
        Date date = new Date(System.currentTimeMillis());
        logger.info(messege + " at time: "+dateFormat.format(date));
    }

}

logs:

[thread 555 at time: 2015/02/18 19:17:35]
[thread 556 at time: 2015/02/18 19:17:40]
[thread 556 at time: 2015/02/18 19:17:41]
[thread 557 at time: 2015/02/18 19:17:44]
[thread 560 at time: 2015/02/18 19:17:48]
[Thread 555 slept well! at time: 2015/02/18 19:19:15]
[Thread 556 slept well! at time: 2015/02/18 19:19:20]
[Thread 556 slept well! at time: 2015/02/18 19:19:21]
[Thread 557 slept well! at time: 2015/02/18 19:19:24]
[Thread 560 slept well! at time: 2015/02/18 19:19:28]

I don't understand what is the metter where I initiate the object? more - it's a string object so anyway it shouldn't be anyway the same reference? what am I misssing?

Edit: Yes, I accidentally swap the code.. that's what happens when you get "your post appears to contain code that is not properly formatted as code" too many times and then find out it was due to log lines :| will fix the swap.

Answer 1

(you probably swapped the code fragments).

I think you are missing an important aspect of Java synchronization. You don't use the field lock as a lock, you assign a lock to an instance. Or as said in the manual:

Unlike synchronized methods, synchronized statements must specify the object that provides the intrinsic lock

So what happens in your second (first in the question code fragment) is that you create a new object, namely:

lock = new String("hello");

And then you lock on that object. That means that every thread has a different instance on which it locks. And so there is no synchronization at all.

EXAMPLE: say you have two threads t1 and t2. First we run a part of thread 1. Thread one calls the method and creates a String instance with value "hello". So the memory looks like:

+------+                    +---------------+
|Action| <------------------+ thread memory |
+------+      +-------+     +---------------+
| lock------->|String |     
+------+      +-------+
              |"hello"|
              +-------+

Next you lock on that object, so - marked lock with a &, you get:

+------+                    +---------------+
|Action| <------------------+ thread memory |
+------+      +-------+     +---------------+
| lock------->|String | &     
+------+      +-------+
              |"hello"|
              +-------+

Now thread t2 comes in. And the first thing t2 does is creating a new object:

+------+                    +----------------+
|Action| <------------------+ thread2 memory |   (old object)
+------+      +-------+     +----------------+   +-------+
| lock------->|String |                          |String | &
+------+      +-------+                          +-------+
              |"hello"|                          |"hello"|
              +-------+                          +-------+

So the new object is not locked, and thread t2 can continue it's execution.

---

It is however possible that you're code will indeed partly lock. It is for instance possible that thread t1 could first create an object, then set Action's field, then get suspended. Then t2 becomes active, also creates an object, sets the Action's field and so both threads end up with the same object, but that's not very likely.

---

A final note is that there is no problem using a "String as a lock". First of all you don't use a String as lock, you should see it as "locking" the string. So the virtual machine simply attaches information to the object that it is locked. There are thus no good or bad objects to get locked.