On 02/27/2014 10:58 AM, David Holmes wrote:
On 27/02/2014 6:38 PM, Peter Levart wrote:
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.

Ah I see - thanks for the analysis. So this is a classic 'barrier' waiting for the other thread to have reached the synchronized method.

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();
     }

Functionally yes this will only return true if the object is locked for >100ms. But it is potentially going to generate a lot of temporary threads. And in theory at least the JIT can optimize the run method into nothing.

Ok, then what about the following:

static boolean isLocked(final Object monitor, final long millis) throws InterruptedException {
        return new Thread() {
            volatile boolean unlocked;

            @Override
            public void run() {
                synchronized (monitor) { unlocked = true; }
            }

            boolean isLocked() throws InterruptedException {
                start();
                join(millis);
                return !unlocked;
            }
        }.isLocked();
    }


// and usage it like:

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


This will allocate about 10 Thread objects per second until the other thread finally reaches the synchronized read() method in BufferedInputStream...


Regards, Peter


Still it is a cute trick :)

David


Regards, Peter


David
-----

Peter


Regards, Peter



On Wed, Feb 26, 2014 at 7:12 AM, Paul Sandoz <paul.san...@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 public static boolean isLocked(final Object monitor, final long millis) throws InterruptedException {
        return new Thread() {
            volatile boolean unlocked;

            @Override
            public void run() {
                synchronized (monitor) { unlocked = true; }
            }

            boolean isLocked() throws InterruptedException {
                start();
                join(millis);
                return !unlocked;
            }
        }.isLocked();
    }

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.





Reply via email to