Why Is It Not a Good Practice to Synchronize on Boolean

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

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.

Is it allowed to synchronize on new Boolean(true)

You are synchronizing on the object reference. Creating private final Object lock = new Object(); would have the same effect as private final static Boolean lock = new Boolean(true);. All you want in this case is a unique object.

You should be careful though, because new Boolean(true) will create a new object reference. If you tried to use Boolean.TRUE or true they are in essence interned and will use the same instance, for example:

Boolean bool1 = new Boolean(true);
Boolean bool2 = Boolean.TRUE;
Boolean bool3 = true;

System.out.println(System.identityHashCode(bool1));
System.out.println(System.identityHashCode(bool2));
System.out.println(System.identityHashCode(bool3));

Will print

1433743869
19203296
19203296

So synchronizing on bool1 will be mutually exclusive to bool2 and bool3 but 2 & 3 will share exclusivity.

Note identityHashCode will give you the reference hash code, that tells you which objects are == equal.

Does boolean variable need synchronization while checking in the given example?

I don't think the code is correct.

You should probably do something like:

while (true) {
  synchronized (mRunLock) {
    if (mRun) break;
  }

  // ...
}

Without this, you don't have a guarantee that writing to mRun happens-before the read in the condition.

It will sort-of work without it, because you are reading mRun inside a synchronized block inside the loop; provided that read is executed, the value will be updated. But the value you read in the loop expression on the next iteration could be the same value as was read on the previous iteration in the synchronized (mRunLock) { if (mRun) doDraw(c); }.

Critically, it isn't guaranteed to read an up-to-date value on the initial iteration. If false is cached, the loop won't execute.

Making mRun volatile would be easier than using synchronization, though.

Need to synchronize setting of boolean member vars?

In the case of reading and writing a primitive like bool or int declaring them as volatile will be plenty. When one threads read the other thread would have finished writing. The variable will never be in an invalid state.

It's probably fair to say that on the whole, the volatile keyword in
Java is poorly documented, poorly understood, and rarely used. To make
matters worse, its formal definition actually changed as of Java 5.
Essentially, volatile is used to indicate that a variable's value will
be modified by different threads.

Declaring a volatile Java variable means:

The value of this variable will
never be cached thread-locally: all
reads and writes will go straight to
"main memory";

Access to the variable acts as
though it is enclosed in a
synchronized block, synchronized on
itself.

We say "acts as though" in the second point, because to the programmer
at least (and probably in most JVM implementations) there is no actual
lock object involved.

http://www.javamex.com/tutorials/synchronization_volatile.shtml

Does access/write to Boolean object needs synchronization

No, Boolean access is NOT atomic (on the level of machine code), although it does take "only 1 operation in Java".

Therefore, yes, you do need synchronization for Boolean.

Please see slides 4-6 of this presentation for code examples.

On a related note, you should not synchronize on a Boolean

Dangers of simultaneous write and read of a boolean in a simple situation

Data races result in undefined behavior. As far as the standard is concerned, a conforming implementation is permitted to segfault.

In practice the main danger is that without synchronization, the compiler will observe enough of the code in the reader loop to judge that b "never changes", and optimize out all but the first read of the value. It can do this because if it observes that there is no synchronization in the loop, then it knows that any write to the value would be a data race. The optimizer is permitted to assume that your program does not provoke undefined behavior, so it is permitted to assume that there are no writes from other threads.

Marking b as volatile will prevent this particular optimization in practice, but even on volatile objects data races are undefined behavior. Calling into code that the optimizer "can't see" will also prevent the optimization in practice, since it doesn't know whether that code modifies b. Of course with link-time/whole-program optimization there is less that the optimizer can't see, than with compile-time-only optimization.

Anyway, preventing the optimization from being made in software doesn't prevent the equivalent thing happening in hardware on a system with non-coherent caches (at least, so I claim: other people argue that this is not correct, and that volatile accesses are required to read/write through caches. Some implementations do behave that way). If you're asking about what the standard says then it doesn't really matter whether or not the hardware shows you a stale cache indefinitely, since behavior remains undefined and so the implementation can break your code regardless of whether this particular optimization is the thing that breaks it.

Using AtomicBoolean instead of synchronized blocks

In your example, a simple volatile boolean would be enough, since you only seem to be doing atomic operations. AtomicBoolean is useful if you need the methods such as compareAndSet.

So in answer to your question, yes, when using a volatile boolean or an AtomicBoolean, other threads will see the updates to the variable.

Synchronization with volatile 'status flag' boolean?

volatile simply ensures that changes to the variable are available to all threads.

The background: a thread may make local copies of shared variables. Synchronizing the values of these local variables with the global shared variables is what volatile effects.

However that does not synchronize in the java sence of a single entry, monitor/critical region.

The entire toolchest of java.util.concurrent offers things like ensuring that only one thread may change the value and such. If you want to start from ground up, one can with two variables do some blocking things: search for Dijkstra algorithms.

Here I think AtomicBoolean might be nice for non-blocking usage.

If you want to achieve to have a global boolean state that pauses resp. resumes threads when toggled (your stopped), instead of some ugly busy wait:

public void run () {
    while (true) {
        doWork(); 
        try {
            barrier.await();
        } catch (InterruptedException | BrokenBarrierException ex) {
            return;
        }
    }
}

Using a global CyclicBarrier - not the nicest API as it works with N predefined Runnables.

Why not lock on a value-based class

Here's what a Blog post by Nicolai Parlog says about value-based classes:

In Java 8 value types are preceded by value-based classes. Their precise relation in the future is unclear but it could be similar to that of boxed and unboxed primitives (e.g. Integer and int). Additionally, the compiler will likely be free to silently switch between the two to improve performance. Exactly that switching back and forth, i.e. removing and later recreating a reference, also forbids identity-based mechanisms to be applied to value-based classes.

So what Nicolai is saying is this:

In the future, compilers may do things that transparently translate between values and value-based classes in ways that do not preserve object identity.
Certain things ("identity-based mechanisms") depend on object identity. Examples include the semantics of == for references, identity hashcode, primitive locking, and object serialization.
For those things, there is the potential that the transparent translation won't be transparent.

In the case of primitive locking, the concern is that something like the following sequence may occur.

An instance of a value-based class is created.
The instance is converted to a value behind the scenes.
The value is then converted back, giving a different object.

If two threads then use "the instance" as a primitive lock, they could be unaware that in fact there are in fact two objects (now). If they then attempted to synchronize, they would (could) be locking different objects. That would mean there was no mutual exclusion on whatever the state was that the locking was intended to protect.

If you don't lock on a value-based class, you won't have to worry about that potential hazard ... in the future.

But note, that Nicolai's blog posting is one person's speculation on what might happen in Java 10 or later.

BTW, I understand the reasons not to lock on Integers and other primitive-wrapper classes; they may be cached.

Caching is not the problem per se, but a mechanism that gives rise to the problem. The real problem is that it is difficult to reason about the object identity of the lock object, and hence whether the locking regime is sound.

With the the primitive wrappers, it is the semantics of boxing and unboxing that gives rise uncertainty of object identity. Going forward, the mooted value type <-> object conversion would be another source of this uncertainty.

The above blog is based on "State of the Values" April 2014. John Rose, Brian Goetz, and Guy Steele which talks about adding value types to a future version of Java. This note is a position statement rather than a fully spec'd (and adopted) proposal. However the note does give us this hint:

"Many of the above restrictions correspond to the restrictions on so-called value-based classes. In fact, it seems likely that the boxed form of every value type will be a value-based class."

which could be read as implying that there will be a relationship between value types and existing value-based classes. (Especially if you read between the lines of the Java 8 description of value-based classes.)

UPDATE - 2019/05/18

Value types didn't make it into Java 12, and they are not (yet) on the list for Java 13.

However, it is already possible to demonstrate a problem that is related to the problem that the blog post talks about:

    public class BrokenSync {
        private final Integer lock = 1;

        public void someMethod() {
            synchronized (lock) {
                // do something
            }
        }
    }

The problem is that each instance of BrokenSync will create an Integer instance by auto-boxing 1. But the JLS says that Integer objects produced by auto-boxing are not necessarily distinct objects. So, you can end up with all instances of BrokenSync using the same Integer object as a lock.

Why Is It Not a Good Practice to Synchronize on Boolean