# V6070. Unsafe synchronization on an object.

The analyzer has detected synchronization on an object that can lead to hidden concurrency issues because the synchronized object may be used implicitly in other logically unrelated parts of the program.

The problem is that if you synchronize on:

- 'this';
- objects of integer wrapper classes (Byte, Short, Integer, Long),
- a Boolean object,
- a String object,

it may result in potential deadlocks and nondeterministic behavior.

This may happen because such objects can be reused in different parts of the program.

The essence of the problem with synchronization by the given objects is that the object used for locking is shared. Such an object can be used for locking in another place without the knowledge of the developer who used the object for locking for the first time. This, in turns, leaves open the likelihood of a deadlock on the same object.

Let's consider a synthetic example of a deadlock when synchronizing by 'this':

    class SynchroThis
    {
      void doSmt()
      {
        synchronized(this)
        {
          // do smt
        }
      }
    }

    ....
    SynchroThis obj = new SynchroThis();
    synchronized(obj)
    {
      Thread t = new Thread(() -> obj.doSmt());
      t.start();
      t.join();
    }
    ....

As a result, the program will never terminate, as a deadlock is occurring by the instance of the SynchroThis class (first deadlock in the main thread by 'obj', the second - in a 't' thread by 'this').

To avoid possible deadlocks, you should use, for example, a private field as an object of locking:

    class A
    {
      private final Object lock = new Object();
      void foo()
      {
        synchronized(lock)
        {
          // do smt
        }
      }
    }

Let's consider a synthetic example of synchronization by an object of the Byte type:

    class FirstClass
    {
      private final Byte idLock;
      ....
      public FirstClass(Byte id, ....)
      {
        idLock = id;
        ....
      }
      ....
      public void calculateFromFirst(....)
      {
        synchronized (idLock)  // <=
        {
          ....
        }
      }
    }

    class SecondClass
    {
      private final Byte idLock;
      ....
      public SecondClass(Byte id, ....)
      {
        idLock = id;
        ....
      }
      ....
      public void calculateFromSecond(....)
      {
        synchronized (idLock)  // <=
        {
          ....
        }
      }
    }

Suppose that Thread N1 works with an object of class 'FirstClass' and Thread N2, with an object of class 'SecondClass'.

Now consider the following scenario:

- The field 'idLock' of the object of class 'FirstClass' has the value 100, and the same is true for the object of class 'SecondClass';
- Thread N1 starts to execute the 'calculateFromFirst' method and keeps running for some time;
- Thread N2 starts (immediately afterward) to execute the 'calculateFromSecond' method.

So, we have two different threads executing completely different logic for different objects. What do we get as a result? Thread N2 will be waiting for Thread N1 to finish executing in the synchronized block on the object 'idLock'. Why does it happen?

Like every object, variables created using wrapper classes will be stored in the heap. Each such object will have its own address in the heap. But there is one tricky detail that must be always taken into account. Integer wrapper classes created using autoboxing with the value within the range [-128..127] are cached by the JVM. That is why such wrappers with identical values within that range will be in fact references to the same object.

This is what has happened in our example. The synchronization is done on the same object in memory, which is an absolutely unexpected behavior.

Besides integer wrapper classes, you should also avoid synchronizing on objects of the following classes:

- Boolean;
- String (as synchronization may happen to occur on the string stored in the string pool).

Synchronizing on such objects is unsafe. It's recommended to use the above method with a private field. But if for any reason it doesn't suit you, you should explicitly create objects using a constructor. This guarantees that the objects will have different addresses. Here is an example of safe code:

    class FirstClass
    {
      private final Byte idLock;
      ....
      public FirstClass(Byte id, ....)
      {
        idLock = new Byte(id);
        ....
      }
      ....
      public void calculateFromFirst(....)
      {
        synchronized (idLock)
        {
          ....
        }
      }
    }
    ....

More information here.

This diagnostic is classified as:

CWE-662 CERT-LCK01-J