Why is reason that synchronized lock not working on String concatenation

Question

I am using the following code. I am not able to achieve synchronization. As per me String pool concept should have worked here.

I want to know the reason behind the problem, not the alternatives to it.

import java.util.Date;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

public class SynchronizationBug {
    private static Map<String,Date> concurrentMap=new ConcurrentHashMap();
    
    public static void start(String processId) throws InterruptedException{
        final String lock="log"+processId;
        synchronized (lock) {
            if(concurrentMap.containsKey(processId)) {
                System.out.println("Process is already working and started at "+concurrentMap.get(processId));
                return;
            }       
            concurrentMap.put(processId,new Date());
        }
    }
    
    
    public static void main(String[] args) throws InterruptedException {
//      System.out.println(isPrime(10));
        final String pId="p1";
        Thread t1=new Thread(new Runnable() {
            @Override
            public void run() {
                // TODO Auto-generated method stub
                try {
                    start(pId);
                } catch (InterruptedException e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                }
            }
        });
        
        Thread t2=new Thread(new Runnable() {

            @Override
            public void run() {
                // TODO Auto-generated method stub
                try {
                    start(pId);
                } catch (InterruptedException e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                }
            }
        });
        
        t1.start();
        t2.start();
        t1.join();
        t2.join();
        
        for (String s:concurrentMap.keySet()) {
            System.out.println(s+" - "+concurrentMap.get(s));
        }
    }
    
}

If I take a lock on processId which is passed as a method parameter, then things are working as expected. What is the reason for this behaviour of Java. I have tested this on Java8 Eclipse IDE.

EDIT: If String pool concept has no relevance then how does the following code work fine.

import java.util.Date;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

public class SynchronizationBug {
    private static Map<String,Date> concurrentMap=new ConcurrentHashMap();
    
    public static void start(String processId) throws InterruptedException{
        final String lock="log"+processId;
        synchronized (processId) {
            if(concurrentMap.containsKey(processId)) {
                System.out.println("Process is already working and started at "+concurrentMap.get(processId));
                return;
            }       
            concurrentMap.put(processId,new Date());
        }
    }
    
    
    public static void main(String[] args) throws InterruptedException {
//      System.out.println(isPrime(10));
        final String pId1="p1";
        final String pId2="p1";
        Thread t1=new Thread(new Runnable() {
            @Override
            public void run() {
                // TODO Auto-generated method stub
                try {
                    start(pId1);
                } catch (InterruptedException e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                }
            }
        });
        
        Thread t2=new Thread(new Runnable() {

            @Override
            public void run() {
                // TODO Auto-generated method stub
                try {
                    start(pId2);
                } catch (InterruptedException e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                }
            }
        });
        
        t1.start();
        t2.start();
        t1.join();
        t2.join();
        
        for (String s:concurrentMap.keySet()) {
            System.out.println(s+" - "+concurrentMap.get(s));
        }
    }
    
}

Here I am passing two different string objects with same value.Due to their same reference in the pool the locking is working fine . Please correct me....

Where do you see a _string pool concept_ here? In fact, you are obviously synchronizing on a different object, each time your method `start` is called. — Seelenvirtuose, Jul 11 '20 at 12:41
_"As per me String pool concept should have worked here."_ Why do you think that? Because it doesn't. — Mark Rotteveel, Jul 11 '20 at 15:38
The reason for the "edit" code works is that you're locking on `processId`, not `lock`. It's not because you're passing two different string objects, because - and this actually is because of the string pool - `pId1` and `pId2` have the same value; and that's the value you are locking on. — Andy Turner, Jul 13 '20 at 08:11

Andy Turner · Accepted Answer · 2020-07-13T08:07:55.893

The reason for this behaviour is that String concatenation, if the operands are not both compile-time constant expressions, results in a new instance of String being created. See here in the language spec:

The String object is newly created (§12.5) unless the expression is a constant expression (§15.29).

Method parameters are not compile-time constant expressions, since the method can be called with any value for the parameter.

And, because the current thread is the only thread which has access to this newly-created String, synchronizing on it has no effect.

Since you say you don't want to know the alternatives, I'll stop there.

You seem unconvinced, despite my pointing to the relevant part of the spec, that the strings you are synchronizing on are different. Try adding this line, right before the synchronized:

    System.out.println(System.identityHashCode(lock));  // Add this
    synchronized (lock) {
      // ...

This will print out the "identity hash code" of the lock, which isn't the same as lock.hashCode(), which is based on the value of the string. This will show that synchronized is synchronizing on different values (unless you're exceptionally lucky/unlucky, since hash collisions are unlikely but possible).

Why is reason that synchronized lock not working on String concatenation

1 Answers1