1

I am well aware that it is not good practice to synchronize on Boolean. There is a lot explanation why it's not good such as:

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

http://telliott.io/node/40/

https://www.securecoding.cert.org/confluence/display/java/LCK01-J.+Do+not+synchronize+on+objects+that+may+be+reused

etc.

So, it is clear that this is BAD practice:

The code synchronizes on a boxed primitive constant, such as an Boolean.

private static Boolean inited = Boolean.FALSE; 
...
    synchronized(inited) 
    { 
        if (!inited) 
        { 
            init(); 
            inited = Boolean.TRUE; 
        } 
    } 
... 

What interests me is what happens if we create final static Boolean with "new" operator, i.e. someting like this (this is from real code I haven't wrote but I maintain it, names of methods etc. are changed):

private final static Boolean lock = new Boolean(true);
...
  public static SomethingGeneric getSomething()
  {
    synchronized(lock)
    {
      if (somethingElse == null)
      {
        try
        {
          somethingElse = persistence.valueobject.getSomeValue(GET_THAT);
          System.out.println("blah blah");
        }
        catch (ObjectCreationException oce)
        {
          // report the error
          log.error("There was this and that error", oce);
          System.out.println("Could not create it");
        }
      }
      return somethingElse;
    }
  }

Would it be then "legal" to use it? Same as if we used Object such as in:

private final Object lock = new Object();

or

private static final Object lock = new Object();

public void doSomething() {
  synchronized (lock) {
    // ...
  }
}
Community
  • 1
  • 1
Nenad Bulatović
  • 7,238
  • 14
  • 83
  • 113
  • 2
    I don't see why not. After all, `new Boolean() instanceof Object == true`. – Siguza Sep 08 '15 at 15:54
  • Usiing a `static final` for the lock is somewhat pointless - you may synchronize on the class object to yield the same locking semantics (`synchronized(SomethingGeneric.class) {...} `) – Gyro Gearless Sep 08 '15 at 16:07
  • If you explicitly create a lock object, you can also go with "new Object()", or if you want to make better readabilty, create a class LockObject (with empty body) for that purpose and use that type. new Boolean() works semantically the same, but readers of your code will always be wondering why Boolean has been chosen for the purpose. – Durandal Sep 08 '15 at 16:26

4 Answers4

3

Thats fine. There are two problems with your first snippet, not that it is a Boolean but first, the object changes in this line:

 inited = Boolean.TRUE;

So some threads can be synchronized on the old object and some on the new. This means, there is no synchronization between the two groups.

And second, Boolean.TRUE and Boolean.FALSE are global objects and if they are used in a similar way somewhere else this will create some hard to detect synchronisation problems (thanks to Mark Rotteveel for pointing it out).

Henry
  • 42,982
  • 7
  • 68
  • 84
  • Sometimes that effect (locking on two different objects) may be intentional, however the larger problem when locking on `Boolean.TRUE` (or `Boolean.FALSE`) is that you are locking on a public static final that might also be abused in other areas, that may lead to very interesting and hard to debug deadlocks. – Mark Rotteveel Sep 08 '15 at 16:48
2

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.

John Vint
  • 39,695
  • 7
  • 78
  • 108
2

Since new Boolean(...) gives you a brand-new Object, the only difference between using new Boolean(...) and new Object() for your lock would be the fact that Boolean has a true or false value associated with it, while Object does not.

You can check that you get new objects from calling new Boolean(...) by running this code snippet:

Boolean bt = Boolean.TRUE;
Boolean bf = Boolean.FALSE;
for (int i = 0 ; i != 20 ; i++) {
    Boolean b = new Boolean(i % 2 == 0);
    System.out.println(b);
    System.out.println("==true : "+(b==bt));
    System.out.println("==false: "+(b==bf));
}

This prints

true
==true : false
==false: false
false
==true : false
==false: false
true
==true : false
==false: false
false
==true : false
==false: false
...

meaning that the objects you get are not the same as Boolean.TRUE and Boolean.FALSE.

Note: Although the practice appears completely harmless, I see no point in trying to synchronize on a Boolean, an Integer, a String, or any other immutable object, because it gives you no advantages over synchronizing on Object lock = new Object(), while not being as recognizable an idiom. This reflects on readability of your code, because programmers reading your code would be scratching their heads over your decision to lock on new Boolean(...).

Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • Would you please add a little more explanation about advantages of synchronizing on Object lock = new Object() over synchronize on a Boolean, an Integer, a String? – Nenad Bulatović Sep 08 '15 at 16:09
  • 2
    @NenadBulatovic The main difference is readability: `lock = new Object()` is instantly recognizable as an idiom by most programmers who dealt with safe synchronization, while `lock = new Boolean(...)` lacks such recognition. On top of that, some programmers will doubt that you know what you are doing until they run a search on Stack Overflow, and see answers to this very question ;-) – Sergey Kalinichenko Sep 08 '15 at 16:20
2

Since new Boolean(true) != new Boolean(true), you can use such a lock.

Be careful though since Boolean.valueOf(true) == Boolean.valueOf(true).

All together if you are maintaining that code, just replace the lock with lock = new Object() as you suggest.

Jean Logeart
  • 52,687
  • 11
  • 83
  • 118