Unsafe: removing the monitorEnter/monitorExit/tryMonitorEnter methods

[David M. Lloyd]

Yes, that would necessarily be the contract of a Monitors class, just as 
it is part of the contract of Lock today.  If your argument is that it 
shouldn't be allowed because it might be used wrong, we might as well 
just delete most of the JDK, ReentrantLock included, since it suffers 
from the exact same potential problem.  The difference is that monitors 
have a simple API in the form of synchronized that people were in the 
past and would continue to be free (and recommended) to use.

I would not call recommending monitors to be a "premature optimization". 
  The memory overhead of ReentrantLock is far too large for many use 
cases, especially when it comes to fine-grained locking of large data 
sets.  In fact I would *only* recommend Lock if your need for the extra 
behavior it provides (i.e., multiple conditions, tryLock, timed tryLock, 
and/or interruptible locking) or for the extra behavior provided by 
ReentrantLock itself (i.e., lock status introspection) outweighs the 
cost of its memory consumption, which so far has happened zero times in 
any of the many frameworks I maintain.

On the other hand, adding some of the missing functionality to a 
Monitors class would be pretty handy, especially if doing so doesn't 
create a large divergence from the existing implementation.  Given that 
tryMonitorEnter already exists, implementing that method on a Monitors 
class seems fairly trivial, which already takes care of one out of the 
four special use cases for Locks.  Interruptible locking would be my 
first choice for a number two, if this idea were actually officially on 
the table.  Finally, an isMonitorHeld() method would tie in another 
branch of the discussion nicely and seems trivially possible as well. 
Any other information would be a purely

It's worth noting that it's already possible for user code to implement 
the basic functionality using JNI methods, though in the interest of 
full disclosure it's worth noting that the enter/exit monitor JNI 
functions are forbidden by spec to interoperate with the corresponding 
bytecodes.

FWIW using the word "monitor" instead of "lock" may help avoid confusion 
and "scare off" those pesky undergrads.

On 03/02/2014 12:48 PM, Dr Heinz M. Kabutz wrote:
> Hi Dave,
>
> "lighter" is subject to the HotSpot Profiler doing the optimizations
> that you expect.  Problem is, once you begin manually locking /
> unlocking monitors, the byte codes are not recognizable as standard
> synchronized(monitor) {} code, thus the optimizations will also not be
> as good (or completely absent).  At least that has been my experience
> when I tried it out a few years ago.  So the argument that synchronized
> is faster than ReentrantLock, besides this being IMHO a premature
> optimization, cannot be directly applied to when we manually do use the
> monitorEnter/monitorExit methods.
>
> The reason why I personally think they are more dangerous than
> ReentrantLock is that we are used to synchronized() {} taking care of
> the unlocking for us.  If we see a thread that is in the BLOCKED state
> (which only happens with synchronized) then we would typically assume
> that another thread is /currently /holding the monitor.
>
> Take the following example code:
>
> import sun.misc.*;
>
> import java.lang.reflect.*;
>
> public class MonitorMystery {
>    public static Unsafe getUnsafe() {
>      try {
>        for (Field field : Unsafe.class.getDeclaredFields()) {
>          if (Modifier.isStatic(field.getModifiers())) {
>            if (field.getType() == Unsafe.class) {
>              field.setAccessible(true);
>              return (Unsafe) field.get(null);
>            }
>          }
>        }
>        throw new IllegalStateException("Unsafe field not found");
>      } catch (Exception e) {
>        throw new IllegalStateException(
>            "Could not initialize unsafe", e);
>      }
>    }
>
>    public static void main(String... args) throws InterruptedException {
>      Thread t = new Thread() {
>        public void run() {
>          getUnsafe().monitorEnter(MonitorMystery.class);
>        }
>      };
>      t.start();
>      Thread.sleep(100);
>      System.out.println("Trying to synchronize");
>      synchronized (MonitorMystery.class) {
>        System.out.println("Managed to synchronized");
>      }
>    }
> }
>
> We now see that we never manage to synchronize in the main thread and in
> fact if you do a thread dump you will see a deadlock involving only a
> single thread :-)
>
> This is why it is so important to use the correct idioms.  In my
> experience, there are very few Java programmers who get this right.
> Brian Goetz's book is correct, but there are other books out there that
> do a shocking job of mangling the idiom.
>
> Whenever possible, use synchronized() { }.  If not, IMHO it would be
> preferable to switch to ReentrantLock.
>
> Regards
>
> Heinz
> --
> Dr Heinz M. Kabutz (PhD CompSci)
> Author of "The Java(tm) Specialists' Newsletter"
> Oracle Java Champion 2005-2013
> JavaOne Rock Star Speaker 2012
> http://www.javaspecialists.eu
> Tel: +30 69 75 595 262
> Skype: kabutz
>
>
>
> David M. Lloyd wrote:
>> Making the language Kindergarten-friendly at the cost of general
>> usefulness is a mistake, IMO.  And anyway there's nothing that is less
>> safe about a Monitors class than ReentrantLock; on the other hand,
>> monitors continue to be considerably lighter (size and (for most of
>> the history of JUC) speed) by every measurement I've ever made.  I
>> would advise monitors over ReentrantLock 9 times out of 10 in any of
>> our code.
>>
>> I just don't think your metaphors - neither of monitor methods being
>> dangerous, nor of Java developers being infants - are really apt.
>>
>> On 03/02/2014 02:51 AM, Dr Heinz M. Kabutz wrote:
>>> With a curious 9 months old crawling around the house, I've just moved
>>> the sharp knives to the top draw in the kitchen - out of reach.  I don't
>>> think we should be encouraging people to use monitor.tryLock() for
>>> various reasons:
>>>
>>> 1. We have a richer interface with Lock/ReentrantLock, including better
>>> Condition that allow easier timed wait idioms.
>>>
>>> 2. It is just too easy to get the idioms wrong for Lock.lock() and
>>> Lock.unlock().  Every time I show this idiom some people in the audience
>>> start arguing with me:
>>>
>>> lock.lock();
>>> try {
>>>   // do something
>>> } finally {
>>>   lock.unlock();
>>> }
>>>
>>> IMHO, this is really an edge case that might be useful to have
>>> semi-accessible at some point, but not something that should generally
>>> be done.  It belongs in the same draw as the sharp knives and the
>>> ability to cause arbitrary asynchronous exceptions (which has been made
>>> more difficult to do in Java 8).
>>>
>>> Brian Goetz wrote:
>>>> Except that Lock has features that are not supported by intrinsic
>>>> locks (timed wait, interruptible wait.)  So the Lock returned would
>>>> not conform to Lock's contract, and attempting to call these methods
>>>> would probably throw UOE.
>>>>
>>>> On 2/27/2014 6:12 AM, Stephen Colebourne wrote:
>>>>> On 26 February 2014 20:54, Martin Buchholz <martinrb at google.com>
>>>>> wrote:
>>>>>> It does seem that being able to tell whether a java object monitor is
>>>>>> currently locked is useful for debugging and monitoring - there
>>>>>> should be a
>>>>>> way to do that.
>>>>>
>>>>> Perhaps it would be useful to be able to expose a java object monitor
>>>>> as an instance of Lock?
>>>>>
>>>>> Lock lk = Lock.ofMonitor(object)
>>>>> if (lk.tryLock()) {
>>>>>    ...
>>>>> }
>>>>>
>>>>> Such a method feels like it would be a useful missing link between
>>>>> synchronized and locks.
>>>>>
>>>>> Stephen
>>>>>
>>>>
>>
>>


-- 
- DML