RFC: 8229160: Reimplement JvmtiRawMonitor to use PlatformMonitor

[Daniel D. Daugherty]

Hi David,

I've finally gotten back to this email thread...

> FYI testing to date:
>  - tiers 1 -3 all platforms
>  - hotspot: serviceability/jvmti
>                           /jdwp
>             vmTestbase/nsk/jvmti
>                           /jdwp
>  - JDK: com/sun/jdi 

You should also add:

open/test/hotspot/jtreg/vmTestbase/nsk/jdb
open/test/hotspot/jtreg/vmTestbase/nsk/jdi
open/test/jdk/java/lang/instrument

I took a quick look through the preliminary webrev and I don't see
anything that worries me.

Re: Thread.interrupt() and raw_wait()

It would be good to see if that semantic is being tested via the
JCK test suite for JVM/TI.

I also very much like/appreciate the decoupling of JvmtiRawMonitors
from ObjectMonitors... Thanks for tackling this crazy task.

Dan



On 8/15/19 2:22 AM, David Holmes wrote:
> Bug: https://bugs.openjdk.java.net/browse/JDK-8229160
>
> Preliminary webrev (still has rough edges): 
> http://cr.openjdk.java.net/~dholmes/8229160/webrev.prelim/
>
> Background:
>
> We've had this comment for a long time:
>
>  // The raw monitor subsystem is entirely distinct from normal
>  // java-synchronization or jni-synchronization.  raw monitors are not
>  // associated with objects.  They can be implemented in any manner
>  // that makes sense.  The original implementors decided to piggy-back
>  // the raw-monitor implementation on the existing Java objectMonitor 
> mechanism.
>  // This flaw needs to fixed.  We should reimplement raw monitors as 
> sui-generis.
>  // Specifically, we should not implement raw monitors via java monitors.
>  // Time permitting, we should disentangle and deconvolve the two 
> implementations
>  // and move the resulting raw monitor implementation over to the 
> JVMTI directories.
>  // Ideally, the raw monitor implementation would be built on top of
>  // park-unpark and nothing else.
>
> This is an attempt to do that disentangling so that we can then 
> consider changes to ObjectMonitor without having to worry about 
> JvmtiRawMonitors. But rather than building on low-level park/unpark 
> (which would require the same manual queue management and much of the 
> same complex code as exists in ObjectMonitor) I decided to try and do 
> this on top of PlatformMonitor.
>
> The reason this is just a RFC rather than RFR is that I overlooked a 
> non-trivial aspect of JvmtiRawMonitors: like Java monitors (as 
> implemented by ObjectMonitor) they interact with the Thread.interrupt 
> mechanism. This is not clearly stated in the JVM TI specification [1] 
> but only in passing by the possible errors for RawMonitorWait:
>
> JVMTI_ERROR_INTERRUPT    Wait was interrupted, try again
>
> As I explain in the bug report there is no way to build in proper 
> interrupt support using PlatformMonitor as there is no way we can 
> "interrupt" the low-level pthread_cond_wait. But we can approximate 
> it. What I've done in this preliminary version is just check interrupt 
> state before and after the actual "wait" but we won't get woken by the 
> interrupt once we have actually blocked. Alternatively we could use a 
> periodic polling approach and wakeup every Nms to check for interruption.
>
> The only use of JvmtiRawMonitors in the JDK libraries (JDWP) is not 
> affected by this choice as that code ignores the interrupt until the 
> real action it was waiting for has occurred. The interrupt is then 
> reposted later.
>
> But more generally there could be users of JvmtiRawMonitors that 
> expect/require that RawMonitorWait is responsive to Thread.interrupt 
> in a manner similar to Object.wait. And if any of them are reading 
> this then I'd like to know - hence this RFC :)
>
> FYI testing to date:
>  - tiers 1 -3 all platforms
>  - hotspot: serviceability/jvmti
>                           /jdwp
>             vmTestbase/nsk/jvmti
>                           /jdwp
>  - JDK: com/sun/jdi
>
> Comments/opinions appreciated.
>
> Thanks,
> David
>
> [1] 
> https://docs.oracle.com/en/java/javase/11/docs/specs/jvmti.html#RawMonitorWait