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.

What do you mean by "when I initialize the method". Can you post the code that creates and starts the threads for the two cases? — Chetan Kinger, Feb 18 '15 at 18:46
In your first example your `myAction` method creates each time separate lock object, so I doesn't stop other threads from using it since all of them will have new locks when using this method. In second case only one thread will be able to enter synchronized block because there will exist only one lock created before method could be invoked. In other words you can remove `lock` and synchronization mechanism from first code example and you should have same results. — Pshemo, Feb 18 '15 at 18:49
@Moria: did you - by accident - swap the two code fragments? — Willem Van Onsem, Feb 18 '15 at 19:07
You can do `Object lock = new Object()`, too. It avoids the "not recommended" bit of Strings. — David Ehrmann, Feb 18 '15 at 19:20
@DavidEhrmann: to be honest I don't see why a String is not recommended as a `lock` object. Since `Strings` are immutable, it is indeed not very common, but there is reason why not to do so... — Willem Van Onsem, Feb 18 '15 at 19:22
@CommuSoft They are not recommended because you can get in trouble with interning when you don't realize it. Simply using `String s = "foo"` can use a global String while in other instances it doesn't. — John Vint, Feb 18 '15 at 20:21
Yeah, but from the moment you start reassigning objects. You must be careful. The same problems can for instance occur if you would use a *soft FlyWeight* pattern where unfrequently used objects are removed from the Weight. — Willem Van Onsem, Feb 18 '15 at 20:27
I guess I didn't explain the problem well enough. Let's assume you have two classes A and B in the same package. A defines `private final String foo = "foo"` and B defines `private final String foo = "foo"`. Regardless of why someone decided to do this; even though both strings are private and in different classes (completely isolated) *they are the same object reference* so synchronizing on their own copy will collide with the other classes. — John Vint, Feb 18 '15 at 20:50
It's an issue with String interning and the mistake to assume different objects are created. — John Vint, Feb 18 '15 at 20:51
Yeah, I understood that. What I meant to say, is that a `String` is not unique on that part. You can have the same issues with patterns like FlyWeight. The syntactical sugar that is used for strings makes it indeed a bit easier to make mistakes. — Willem Van Onsem, Feb 18 '15 at 21:16
See also https://stackoverflow.com/questions/133988/synchronizing-on-string-objects-in-java — Raedwald, Aug 16 '18 at 09:29

Willem Van Onsem · Answer 1 · 2015-02-18T19:43:23.633

(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.

But what I don't understand is: 1) what does it matter where I initiate? the member isn't static and on both cases t1 and t2 will have each "their own" lock. 2)how can that be that each one of them have a different String? if they are using the same string-value, it should be referred to the same object in memory - isn't it? thanks! — Moria, Feb 19 '15 at 08:45
The point is that you **instantiate a new object every time you call the method**. So instead of having one object shared by all threads, each thread generate its own object. — Willem Van Onsem, Feb 19 '15 at 14:38

synchronized depended on string initilizing

1 Answers1