Unsafe: removing the monitorEnter/monitorExit/tryMonitorEnter methods
Peter Levart
peter.levart at gmail.com
Tue Mar 4 07:06:42 UTC 2014
On 03/04/2014 08:01 AM, Peter Levart wrote:
> On 03/04/2014 05:09 AM, David Holmes wrote:
>> On 4/03/2014 1:45 PM, David Holmes wrote:
>>> On 3/03/2014 10:56 PM, David M. Lloyd wrote:
>>>> 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.
>>>
>>> We should not introduce anything that allows something that was
>>> guaranteed to be safe by the language, to become unsafe. So I can
>>> support a call for tryMonitorEnter, but not for explicit enter/exit
>>> actions.
>>
>> Which of course is not possible at the API level - Doh! Sorry.
>
> Unless the API is made safe by packing it into a lambda-consuming
> method. For example:
>
> public static boolean trySynchronized(Runnable action);
Sorry, forgot the monitor:
public static boolean trySynchronized(Object monitor, Runnable action);
I guess new java.lang.Object instance method is out of question?
Regards, Peter
>
> Regards, Peter
>
>>
>> David
>>
>>> David
>>> -----
>>>
>>>> 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
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>
More information about the core-libs-dev
mailing list