RFC: 8229160: Reimplement JvmtiRawMonitor to use PlatformMonitor
David Holmes
david.holmes at oracle.com
Tue Sep 10 07:54:54 UTC 2019
It turns out that polling for interrupts is actually very difficult to
do correctly. There is an inherent race with timing out and being
signalled that can result in lost signals. Trying to account for that
without introducing other problems would lead to a very complex
synchronization mechanism (which would need to track waiters,
notifications and a "generation" count). The end result is that we'd
break the tie to ObjectMonitor code, but would replace it with new
complex untried code. :(
David
On 10/09/2019 8:49 am, David Holmes wrote:
> Hi Serguei,
>
> On 10/09/2019 4:26 am, serguei.spitsyn at oracle.com wrote:
>> Hi David,
>>
>> On 9/8/19 19:15, David Holmes wrote:
>>> Hi Dan,
>>>
>>> On 7/09/2019 6:50 am, Daniel D. Daugherty wrote:
>>>> Hi David,
>>>>
>>>> I've finally gotten back to this email thread...
>>>
>>> Thanks.
>>>
>>>>> 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
>>>
>>> Okay - in progress. Though I can't see any use of RawMonitors in any
>>> of these tests.
>>>
>>>> I took a quick look through the preliminary webrev and I don't see
>>>> anything that worries me.
>>>
>>> Thanks. I'll prepare a more polished webrev soon.
>>>
>>>> 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.
>>>
>>> It isn't. The only thing directly tested for RawMonitorWait is normal
>>> successful operation and reporting "not owner" when not the owner. No
>>> check for JVMTI_ERROR_INTERRUPT exists other than as input for the
>>> GetErrorName function.
>>
>> This is most likely true.
>> My only concern is if RawMonitor's can be used in the JCK test
>> libraries (low probability).
>> I've asked Leonid Kuskov (JCK) to double check this (added to the
>> mailing list).
>>
>>>
>>> There's only one test in the whole test base that checks for the
>>> interrupt and that is
>>> vmTestbase/nsk/jvmti/RawMonitorWait/rawmnwait005/. In that test if we
>>> are not interrupted before the RawMonitorWait we will wait until the
>>> full timeout elapses - which is 2 minutes by default - then return
>>> and report the interrupt. Hence the test still passes. (If it was an
>>> untimed wait that would be different of course).
>>
>> I figured the same last Friday.
>> One more place to care about are NSK tests libraries that are located
>> here:
>> test/hotspot/jtreg/vmTestbase/nsk/share
>>
>> There are a couple of places where the RawMonitor is used:
>> jvmti/hotswap/HotSwap.cpp: if
>> (!NSK_JVMTI_VERIFY(jvmti->RawMonitorWait(waitLock, millis)))
>> jvmti/jvmti_tools.cpp: jvmtiError error =
>> env->RawMonitorWait(monitor, millis);
>>
>> The use in HotSwap.cpp is local.
>> The jvmti_tools.cpp defines this:
>> void rawMonitorWait(jvmtiEnv *env, jrawMonitorID monitor, jlong
>> millis) {
>> jvmtiError error = env->RawMonitorWait(monitor, millis);
>> exitOnError(error);
>> }
>>
>> which is used in the jvmti/agent_tools.cpp but does not depend on
>> interrupting of RawMonitor's (as I can see).
>> One more place to mention is:
>> jvmti/DataDumpRequest/datadumpreq001/datadumpreq001.cpp
>>
>> But I see no problems there as well.
>>
>>
>>
>> The JDWP implementation is using RawMonitor's.
>> Please, see functions debugMonitorWait()/debugMonitorTimedWait() in
>> src/jdk.jdwp.agent/share/native/libjdwp/util.c.
>>
>> It expects the JVMTI_ERROR_INTERRUPT but never makes a call to the
>> JVMTI ThreadInterrupt().
>> So, it looks like it does not depend on interrupting of RawMonitor's
>> in any way.
>>
>>>
>>> The more I try to convince people this change should be okay, the
>>> more uncomfortable I get with my own arguments. :) I think I'm going
>>> to implement the polling approach for checking interrupts - say 500ms.
>>
>> The JVMTI spec tells that the JVMTI_ERROR_INTERRUPT can be returned
>> from the RawMonitorWait:
>> https://docs.oracle.com/en/java/javase/11/docs/specs/jvmti.html#RawMonitorWait
>>
>
> Yes it does and that is the only thing that implies a connection to
> Thread.interrupt.
>
>> which means that RawMonitorWait can be interrupted with the
>> Thread.Interrupt()
>> or JVMTI InterruptThread():
>> https://docs.oracle.com/en/java/javase/11/docs/specs/jvmti.html#InterruptThread
>>
>
> That's one way to interpret the fact that RawMonitorWait can return
> JVMTI_ERROR_INTERRUPT, but the actual interaction between
> Thread.interrupt and RawMonitorWait is not explicitly stated. Arguably
> you can just check for interruption before and after the wait, to see
> whether to return JVMTI_ERROR_INTERRUPT, without necessarily being able
> to break out of the wait itself. That's been the whole premise of this
> change proposal - that responsiveness to interrupts is more a
> quality-of-implementation issue.
>
> But in any case I've decided to try the polling approach so that we
> won't wait forever if interrupted but not notified.
>
> Thanks,
> David
> -----
>
>>
>>
>> Thanks,
>> Serguei
>>
>>>> I also very much like/appreciate the decoupling of JvmtiRawMonitors
>>>> from ObjectMonitors... Thanks for tackling this crazy task.
>>>
>>> Thanks :)
>>>
>>> David
>>>
>>>> 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
>>>>>
>>>>
>>
More information about the serviceability-dev
mailing list