Hi Martin,
On 28/08/2019 8:53 pm, Doerr, Martin wrote:
Hi David,
I've run it through our nightly tests and got an assertion on Windows.
Test:
vmTestbase/nsk/jvmti/scenarios/allocation/AP04/ap04t001
Assertion:
# Internal Error (src/hotspot/share/prims/jvmtiRawMonitor.cpp:173), pid=17824,
tid=7808
# assert(__the_thread__->is_VM_thread()) failed: must be VM thread
That's very interesting - the assertion exists in the current code as well.
Current thread (0x00000213a5dbb800): GCTaskThread "GC Thread#0" [stack:
0x0000001bfbe00000,0x0000001bfbf00000] [id=7808]
Now why would a GCTaskThread being executing code that accesses
JvmtiRawMonitors? Are we in some kind of event callback?
Stack: [0x0000001bfbe00000,0x0000001bfbf00000]
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
...
V [jvm.dll+0x50d1a2] report_vm_error+0x102 (debug.cpp:264)
V [jvm.dll+0x8f3996] JvmtiRawMonitor::raw_enter+0x1e6
(jvmtirawmonitor.cpp:173)
V [jvm.dll+0x8d6121] JvmtiEnv::RawMonitorEnter+0x211 (jvmtienv.cpp:3345)
C [ap04t001.dll+0x30ef]
Is there any more stack? What is that dll?
So this assumption is not true:
// No other non-Java threads besides VM thread would acquire
// a raw monitor.
This is the only issue I've seen so far.
As I said this is not a new assertion so its interesting that you fund this.
Can you tell me what test this was and how to reproduce?
Thanks,
David
-----
Best regards,
Martin
-----Original Message-----
From: David Holmes <[email protected]>
Sent: Dienstag, 27. August 2019 00:21
To: [email protected]; serviceability-dev <serviceability-
[email protected]>; [email protected]; [email protected];
[email protected]; Doerr, Martin <[email protected]>
Subject: Re: RFC: 8229160: Reimplement JvmtiRawMonitor to use
PlatformMonitor
On 26/08/2019 6:25 pm, [email protected] wrote:
Hi David,
On 8/20/19 22:21, David Holmes wrote:
Hi Serguei,
On 21/08/2019 9:58 am, [email protected] wrote:
Hi David,
The whole approach looks good to me.
Thanks for taking a look. My main concern is about the interrupt
semantics, so I really need to get some end-user feedback on that
aspect as well.
I don't have any opinion yet on what interrupt semantics tool developers
really need.
Yes, we may need to request some feedback.
I've now explicitly added JC, Yasumasa, Severin and Martin, to this
email thread to try and solicit feedback from all the major players that
seem interested in this serviceability area. Folks I'd really appreciate
any feedback you may have here on the usecases for JvmtiRawMonitors,
and
in particular the use RawMonitorWait and its interaction with
Thread.interrupt.
My gut feeling tells me it is not good to break the original semantics. :)
But let me think about it a little bit more.
Me too, but I wanted to start simple. I suspect I will have to at least
implement time-based polling of the interrupt state.
Also, we need to file a CSR for this.
Depending on how this proceeds, yes.
+ if (jSelf != NULL) {
+ if (interruptible && Thread::is_interrupted(jSelf, true)) {
+ // We're now interrupted but we may have consumed a notification.
+ // To avoid lost wakeups we have to re-issue that notification, which
+ // may result in a spurious wakeup for another thread.
Alternatively we
+ // ignore checking for interruption before returning.
+ notify();
+ return false; // interrupted
+ }
I'm a bit concerned about introduction of new spurious wake ups above.
Some tests can be not defensive against it, so we may discover new
intermittent failures.
That is possible. Though given spurious wakeups are already possible
any test that is incorrectly using RawMonitorWait() without checking a
condition, is technically already broken.
Agreed.
I even think it is even better if spurious wakeups will happen more
frequently.
It should help to identify and fix such spots in the test base.
Yes it is good tests. Alas not so good for production code :)
Not checking for interruption after the wait will also require some
test changes, and it weakens the interrupt semantics even further.
I'm thinking about a small investigation on how this is used in our tests.
There seem to be a few uses that are susceptible to spurious wakeup
errors, but those tests don't use interrupt.
Thanks,
David
Thanks,
Serguei
Thanks,
David
-----
Thanks,
Serguei
On 8/14/19 11:22 PM, 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#RawMo
nitorWait
