Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why is it not a good practice to synchronize on Boolean?

My architect always says that

Never synchronize on Boolean

I am not able to understand the reason why and would really appreciate if someone could explain with an example as to why it is not a good practice. Reference Sample Code

private Boolean isOn = false; private String statusMessage = "I'm off"; public void doSomeStuffAndToggleTheThing(){     // Do some stuff    synchronized(isOn){       if(isOn){          isOn = false;          statusMessage = "I'm off";          // Do everything else to turn the thing off       } else {          isOn = true;          statusMessage = "I'm on";          // Do everything else to turn the thing on       }    } } 
like image 283
Rachel Avatar asked Apr 25 '12 21:04

Rachel


People also ask

What is the disadvantage of synchronization?

The main advantage of synchronization is that by using the synchronized keyword we can resolve the date inconsistency problem. But the main disadvantage of a synchronized keyword is it increases the waiting time of the thread and affects the performance of the system.

What are the risks of synchronization?

Synchronization risk arises from arbitrageur's uncertainty about when other arbitrageurs will start exploiting a common arbitrage opportunity (Abreu and Brunnermeier, 2002 and 2003). The arbitrage opportunity appears when prices move away from fundamental values.

What is good practice in synchronization?

Java Synchronization can cause a deadlock. The best way to avoid this problem is to avoid the use of Java synchronization. One of the most common uses of synchronization is to implement pooling of serially reusable objects. Often, you can simply add a serially reusable object to an existing pooled object.

When should synchronization be used?

Synchronization is needed when Object is mutable. If shared Object is immutable or all the threads which share the same Object are only reading the Object's state not modifying then you don't need to synchronize it. Java programming language provide two synchronization idioms: Methods synchronization.


2 Answers

I am not able to understand the reason why we should "never synchronize on Boolean"

You should always synchronize on a constant object instance. If you synchronized on any object that you are assigning (i.e. changing the object to a new object) then it is not constant and different threads will be synchronizing on different object instances. Because they are synchronizing on different object instances, multiple threads will be entering the protected block at the same time and race conditions will happen. This is the same answer for synchronizing on Long, Integer, etc..

// this is not final so it might reference different objects Boolean isOn = true; ... synchronized (isOn) {    if (isOn) {       // this changes the synchronized object isOn to another object       // so another thread can then enter the synchronized with this thread       isOn = false; 

To make matters worse, any Boolean that is created through autoboxing (isOn = true) is the same object as Boolean.TRUE (or .FALSE) which is a singleton in the ClassLoader across all objects. Your lock object should be local to the class it is used in otherwise you will be locking on the same singleton object that other classes might be locking on in other lock cases if they are making the same mistake.

The proper pattern if you need to lock around a boolean is to define a private final lock object:

private final Object lock = new Object(); ...  synchronized (lock) {    ... 

Or you should also consider using the AtomicBoolean object which means you may not have to synchronize on it at all.

private final AtomicBoolean isOn = new AtomicBoolean(false); ...  // if it is set to false then set it to true, no synchronization needed if (isOn.compareAndSet(false, true)) {     statusMessage = "I'm now on"; } else {     // it was already on     statusMessage = "I'm already on"; } 

In your case, since it looks like you need to toggle it on/off with threads then you will still need to synchronize on the lock object and set the boolean and avoid the test/set race condition:

synchronized (lock) {     if (isOn) {         isOn = false;         statusMessage = "I'm off";         // Do everything else to turn the thing off     } else {         isOn = true;         statusMessage = "I'm on";         // Do everything else to turn the thing on     } } 

Lastly, if you expect the statusMessage to be accessed from other threads then it should be marked as volatile unless you will synchronize during the get as well.

like image 147
Gray Avatar answered Oct 06 '22 04:10

Gray


private Boolean isOn = false; public void doSomeStuffAndToggleTheThing(){    synchronized(isOn){ 

This is a terrible idea. isOn will reference the same object as Boolean.FALSE which is publicly available. If any other piece of badly written code also decides to lock on this object, two completely unrelated transactions will have to wait on each other.

Locks are performed on object instances, not on the variables that reference them:

enter image description here

like image 42
McDowell Avatar answered Oct 06 '22 03:10

McDowell