<div dir="ltr">I had a look through Google's codebase, to see if we have any outlaws who are using these primitive boxes as locks. Of course, almost all locks (over 99.9% in Google's codebase) have a static type of either Object (typically a new Object() created just for its lock), or Class (typically for class-wide exclusion during a static method/initializer). It's not easy for me to discover how often these Objects are actually Integers, but what I could do was at least find cases where an Integer is explicitly used as a lock, to see if any of those are intentional, important use cases that might be worth changing Brian's proposal for.<div><br></div><div>The TL;DR is: We do have a small number of cases using boxes as locks, but none of them are a good idea, and there are easy "workarounds" available if locking on a box became illegal (and which should really be applied anyway). I don't foresee any serious migration issues that we would incur based on these cases - through again, for Integers whose static type is Object, I have no data at this time, and we would probably appreciate some gradual-migration tools like a warning if the VM notices we're taking an ill-advised lock.</div><div><br></div><div>For more information about how I came to this conclusion, read on.</div><div><br></div><div>I searched for all statements of the form: synchronized (expr()) {...}, or a use of wait/notify/notifyAll on an expr(), and asked javac what the static type of expr() is. The only primitive-box types that appear in this list are Boolean, Integer, and Long. The Boolean cases that I looked at all have the same basic structure: someone wants to store a boolean field, but wants to update it in a critical section. Instead of doing the "right" thing and using a separate lock, such as `this` or a dedicated lock object, they just turn their boolean into a Boolean and lock on it while updating it:</div><div><br></div><div>class Flag {</div><div>  Boolean active = false;</div><div>  void update() {</div><div>    synchronized (active) { active = computeNewState(); }</div><div>  }</div><div>}</div><div><br></div><div>This is clearly wrong, and it's easy to fix.</div><div><br></div><div>There were a slightly more varied set of patterns around Integer locks. I identified 3 basic categories of motivations.</div><div><br></div><div>One amusing category is "wrong on purpose", as part of some demonstration of a tool that detects locking problems. If this ever becomes illegal, we can just update the example to use an Object instead:</div><div>      final Integer dummy = new Integer(0);<br>      synchronized (dummy) {<br>        runExample("blocked", new Runnable() {<br>          public void run() {<br>            synchronized (dummy) {<br>            }<br>          }<br>        });<br>        sleep(2000);<br>      }<br></div><div><br></div><div>A second, fairly innocuous, case is from authors who seem unaware that you can use Class or Object for locks. Code in this category creates an Integer for the sole purpose of locking on it, and never reads or writes the field otherwise. In one example, the author notes that they wish they could use new Object() instead, but their serialization framework requires all fields to be serializable. Integer must have looked like an easy solution, but we know there were better options available. A simple example of a class in this category:</div><div>class Counters {</div><div>  private Map<String, String> values;</div><div>  private final Integer valuesLock = 0;</div><div>  void count(String k, String v) {</div><div>    synchronized(valuesLock) {</div><div>      values.put(k, v);</div><div>    }</div><div>  }</div><div>}</div><div><br></div><div>All the above cases share the problem that they introduce unnecessary contention with other objects making the same mistake. The third category I found is worse, because it also has the problem of not ensuring mutual exclusion! The Integer being locked on is reassigned while the lock is held, meaning that other threads could barge in, since they will now be locking on a different object. An example:</div><div>class Sampler {</div><div>  private Integer count = 0;</div><div>  private final int frequency = 100;</div><div>  void maybeEmit(Object sample) {</div><div>    synchronized(count) {</div><div>      count++;</div><div>    }</div><div>    if (count % frequency == 0) {</div><div>      emit(sample);</div><div>    }</div><div>  }</div><div>}</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Feb 19, 2020 at 8:46 AM Brian Goetz <<a href="mailto:brian.goetz@oracle.com">brian.goetz@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">One of the main impediments to migrating the primitive wrappers to be <br>
the reference projection of an inline class is locking -- there may be <br>
code out there that locks on instances of primitive wrappers.  Let's <br>
look at how we might wean users off, and what the compatibility <br>
constraints really are.<br>
<br>
First, note that all the wrappers have had static `valueOf()` factories, <br>
and the corresponding constructors have been deprecated since Java 9.  <br>
It would be reasonable to deprecate these for removal at any time.  <br>
(These would not actually be removed, but instead made private.)<br>
<br>
Having removed the public constructors, now the only way to get your <br>
hands on an Integer is through some mechanism like `valueOf()`.  I claim <br>
now that:<br>
<br>
     Under such a scheme, any code outside of java.lang that <br>
synchronizes on a wrapper is<br>
     inherently broken<br>
<br>
That is, it is synchronizing on an object whose locking protocol you are <br>
not party to, which is not guaranteed non-aliased, and which may in fact <br>
be permanently locked by some other thread. Therefore, it is a <br>
forseeable outcome for such a sync attempt to block forever.<br>
<br>
This was the intent behind <a href="https://openjdk.java.net/jeps/169" rel="noreferrer" target="_blank">https://openjdk.java.net/jeps/169</a>, though it <br>
wasn't obvious from the title -- the ability to mark certain objects as <br>
permanently locked.<br>
<br>
So, concrete proposal:<br>
<br>
  - Move forward with the deprecation-for-removal of the constructors;<br>
<br>
  - Add hortatory notes in the spec for `valueOf()` that locking on <br>
objects whose provenance you do not control is a risky business, and may <br>
well never complete;<br>
<br>
  - Poison instances of primitive wrappers by marking their headers at <br>
creation with a bit pattern that will send locking through the slow <br>
path, where it can see that these are poisoned objects and cause lock <br>
attempts to never complete (or throw)<br>
<br>
  - Provide a JDK-scoped option to turn off the above behavior <br>
(-Xx:AllowStupidBrokenLockingOnPrimitiveWrappers)<br>
<br>
I don't see anything stopping us from doing any of these immediately.<br>
<br>
<br>
<br>
</blockquote></div>