Unsafe: removing the monitorEnter/monitorExit/tryMonitorEnter methods

Peter Levart peter.levart at gmail.com
Thu Feb 27 08:38:59 UTC 2014


On 02/27/2014 08:41 AM, David Holmes wrote:
> On 27/02/2014 5:30 PM, Peter Levart wrote:
>> On 02/27/2014 08:29 AM, Peter Levart wrote:
>>> On 02/26/2014 09:54 PM, Martin Buchholz wrote:
>>>> I don't recall having added this particular wart
>>>> to test/java/lang/ProcessBuilder/Basic.java, and history suggests that
>>>> others did that.
>>>>
>>>> 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.
>>>
>>> Thread.holdsLock(Object) ?
>>
>> Ah, you meant to query from some other thread, right?
>
> Right - that is the usage in Basic.java but I'm still scratching my 
> head trying to understand what finding the BufferedInputStream locked 
> is supposed to signify.

Here's the relevant code block:

                     while (unsafe.tryMonitorEnter(s)) {
                         unsafe.monitorExit(s);
                         Thread.sleep(1);
                     }

This code block waits until it finds that some thread has acquired the 
lock on 's' for at least 1 ms (or else it can miss it and will wait 
further) ...

Since this block is executed for s instanceof BufferedInputStream, what 
this block does, is wait until one of synchronized methods on 
BufferedInputStream is called. The thread that does that, calls read() 
or read(byte[]) and blocks while holding a lock, since the child process 
that is spawned and whose stdout the thread is trying to read in this 
situation does a 10 minute sleep. I think the following could be used 
instead for the same purpose:

             while (!isLocked(s, 100L)) {
                 Thread.sleep(1);
             }

// using the following utility method:

     static boolean isLocked(final Object monitor, long millis) throws 
InterruptedException {
         Thread t = new Thread() {
             @Override
             public void run() {
                 synchronized (monitor) { }
             }
         };
         t.start();
         t.join(millis);
         return t.isAlive();
     }


Regards, Peter

>
> David
> -----
>
>> Peter
>>
>>>
>>> Regards, Peter
>>>
>>>>
>>>>
>>>> On Wed, Feb 26, 2014 at 7:12 AM, Paul Sandoz <paul.sandoz at oracle.com>
>>>> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Out of all the methods on Unsafe i think the
>>>>> monitorEnter/monitorExit/tryMonitorEnter are the least used and are
>>>>> very
>>>>> strong candidates for removal.
>>>>>
>>>>> 99% of use-cases are supported by classes in the
>>>>> java.util.concurrent.locks package.
>>>>>
>>>>>
>>>>> Within the JDK itself it is only used in one JDK test file
>>>>> test/java/lang/ProcessBuilder/Basic.java:
>>>>>
>>>>>                      while (unsafe.tryMonitorEnter(s)) {
>>>>>                          unsafe.monitorExit(s);
>>>>>                          Thread.sleep(1);
>>>>>                      }
>>>>>
>>>>> for a test verifying an EOF is received on pending reads and it is
>>>>> polling
>>>>> to check when the process builder acquires the lock before
>>>>> destroying the
>>>>> process, presumably to avoid some sort of race condition that
>>>>> occasionally
>>>>> causes the test to fail.
>>>>>
>>>>> I believe this test as been through a number of rounds, i stared at
>>>>> things
>>>>> for a bit, but cannot quickly work out a replacement; i lack the
>>>>> knowledge
>>>>> on this area.
>>>>>
>>>>>
>>>>> Outside of the JDK i can only find one usage of monitorExit/Enter
>>>>> (according to grep code) in JBoss modules, and i believe this is
>>>>> only used
>>>>> on Java 1.6 to work around some limitations in class loading that 
>>>>> were
>>>>> fixed in 1.7.
>>>>>
>>>>>
>>>>> Given such very limited use i propose to remove these methods after
>>>>> having
>>>>> worked out a fix for ProcessBuilder/Basic.java test.
>>>>>
>>>>> Paul.
>>>>>
>>>
>>




More information about the core-libs-dev mailing list