JDK-8034785 : Assert when recursively trying to take the monitor from JVMTI MonitorContendedEnter
  • Type: Bug
  • Component: hotspot
  • Sub-Component: jvmti
  • Affected Version: hs25,8,9
  • Priority: P3
  • Status: Open
  • Resolution: Unresolved
  • Submitted: 2014-02-12
  • Updated: 2022-09-20
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbdUnresolved
Related Reports
Relates :  
JDK-7184993 - NPG: assert(Self->_Stalled == 0) failed: invariant
Relates :  
JDK-8035150 - ShouldNotReachHere() in ConstantPool::copy_entry_to
Description
(This was seen when trying to reproduce a crash with the netbeans profiler: https://bz.apache.org/netbeans/show_bug.cgi?id=240763 )

If one writes a JVMTI agent that listens for MonitorContendedEnter, and from that callback tries to take the same lock again, the JVM will assert on objectMonitor.cpp:348:  assert (Self->_Stalled == 0, "invariant")

This is the same problem as described in JDK-7184993 which was closed as a dup of the no-permgen enhancement. The problem is pretty well described in that bug in this comment https://bugs.openjdk.java.net/browse/JDK-7184993?focusedCommentId=12584932&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-12584932

The first question is: Is this valid code? Is it ok for a JVMTI agent to try to acquire locks in MonitorContendedEnter?

Second question: If so, how do we make it possible?
Comments
Targeting to TBD.
04-11-2020
No time to fix it in 11, moved to tbd_major.
29-05-2018
Need to re-target it to 10. It seems there is not enough time to fix it in 9.
29-06-2016
I agree that JVMTI is the one that has to give here. It is allowing arbitrary work in a path which is not that flexible. It seems like the contended enter notification has to be done before you've anything that actually puts you into the blocking path and it needs to detect that it's recursively notifying about contention and simply skip the notification. I was only suggesting that the Stalled field could be used to detect that it's doing recursive notification but it would probably be best to have JVMTI use it's own machinery for that. The current notification point isn't really safely implementable I think. I'm not quite sure what the code does when this occurs in the product bits. Recurse until the contention ends?
02-06-2016
We can not make the synchronization subsystem reentrant just to deal with the fact the JVMTI spec is naive when defining how event notifications should work. Many callbacks need to be severely constrained in what they attempt to do based on the location from which the callback is invoked! The only way to ameliorate that would be too move the points at which the callbacks are invoked, but that would also change the semantics of the event. The assertion that fires in this case is highly desirable as that is how we detect various incorrect attempts to re-enter the locking subsystem (eg when placing logging statements in inappropriate places).
02-06-2016
It seems like the Stalled flag has to be used to disable recursive contention notifications and allow the recursive enter to block and wait to acquire the lock. Not sure how that will work but the monitor contention logic really has be able to handle this since there doesn't seem to be any restriction on what you can do inside the contention notification.
02-06-2016
This happens pretty commonly for me with VisualVM. In my case a FindClass call is contending on a lock and then triggering the JVMTI contended monitor enter notification, which loads classes through FindClass and reenters the lock. V [libjvm.dylib+0x9adb1a] VMError::report_and_die()+0x3f2 V [libjvm.dylib+0x337581] report_vm_error(char const*, int, char const*, char const*)+0x63 V [libjvm.dylib+0x7e2c2f] ObjectMonitor::enter(Thread*)+0x169 V [libjvm.dylib+0x90de46] ObjectSynchronizer::slow_enter(Handle, BasicLock*, Thread*)+0x1b8 V [libjvm.dylib+0x90dc81] ObjectSynchronizer::fast_enter(Handle, BasicLock*, bool, Thread*)+0xe3 V [libjvm.dylib+0x46d4f6] InstanceKlass::link_class_impl(instanceKlassHandle, bool, Thread*)+0x35c V [libjvm.dylib+0x46e713] InstanceKlass::link_class(Thread*)+0x91 V [libjvm.dylib+0x46d942] InstanceKlass::initialize_impl(instanceKlassHandle, Thread*)+0x32 V [libjvm.dylib+0x46d8bd] InstanceKlass::initialize(Thread*)+0x5d V [libjvm.dylib+0x59e9c8] find_class_from_class_loader(JNIEnv_*, Symbol*, unsigned char, Handle, Handle, unsigned char, Thread*)+0x99 V [libjvm.dylib+0x52bba2] jni_FindClass+0x49c C [libprofilerinterface.jnilib+0x1165] initializeMethods+0x36d C [libprofilerinterface.jnilib+0x1797] monitor_contended_enter_hook+0x35 V [libjvm.dylib+0x67ff99] JvmtiExport::post_monitor_contended_enter(JavaThread*, ObjectMonitor*)+0x27d V [libjvm.dylib+0x7e3069] ObjectMonitor::enter(Thread*)+0x5a3 V [libjvm.dylib+0x90de46] ObjectSynchronizer::slow_enter(Handle, BasicLock*, Thread*)+0x1b8 V [libjvm.dylib+0x90dc81] ObjectSynchronizer::fast_enter(Handle, BasicLock*, bool, Thread*)+0xe3 V [libjvm.dylib+0x46d4f6] InstanceKlass::link_class_impl(instanceKlassHandle, bool, Thread*)+0x35c V [libjvm.dylib+0x46e713] InstanceKlass::link_class(Thread*)+0x91 V [libjvm.dylib+0x46d942] InstanceKlass::initialize_impl(instanceKlassHandle, Thread*)+0x32 V [libjvm.dylib+0x46d8bd] InstanceKlass::initialize(Thread*)+0x5d V [libjvm.dylib+0x59e9c8] find_class_from_class_loader(JNIEnv_*, Symbol*, unsigned char, Handle, Handle, unsigned char, Thread*)+0x99 V [libjvm.dylib+0x52bba2] jni_FindClass+0x49c C [libprofilerinterface.jnilib+0x1165] initializeMethods+0x36d C [libprofilerinterface.jnilib+0x1992] Java_org_netbeans_lib_profiler_server_system_Classes_setWaitTrackingEnabled+0x19 j org.netbeans.lib.profiler.server.system.Classes.setWaitTrackingEnabled(Z)Z+0 j org.netbeans.lib.profiler.server.ProfilerInterface.disableProfilerHooks()V+1 j org.netbeans.lib.profiler.server.ProfilerServer.doActivate(I)V+0 j org.netbeans.lib.profiler.server.ProfilerServer.access$000(I)V+1 j org.netbeans.lib.profiler.server.ProfilerServer$AttachDynamicThread.run()V+4 v ~StubRoutines::call_stub V [libjvm.dylib+0x4eda6f] JavaCalls::call_helper(JavaValue*, methodHandle*, JavaCallArguments*, Thread*)+0xf0b V [libjvm.dylib+0x4eb516] JavaCalls::call_virtual(JavaValue*, KlassHandle, Symbol*, Symbol*, JavaCallArguments*, Thread*)+0x500 V [libjvm.dylib+0x4eb7d0] JavaCalls::call_virtual(JavaValue*, Handle, KlassHandle, Symbol*, Symbol*, Thread*)+0x76 V [libjvm.dylib+0x5b8c73] thread_entry(JavaThread*, Thread*)+0xad V [libjvm.dylib+0x947928] JavaThread::thread_main_inner()+0x178 V [libjvm.dylib+0x9476f5] JavaThread::run()+0x357 V [libjvm.dylib+0x7f5a35] java_start(Thread*)+0xf9 C [libsystem_pthread.dylib+0x399d] _pthread_body+0x83 C [libsystem_pthread.dylib+0x391a] _pthread_body+0x0 C [libsystem_pthread.dylib+0x1351] thread_start+0xd
01-06-2016
This is my analysis below. The test java code in the Recursive.java is: public class Recursive { static Object lock = new Recursive(); static class MyThread extends Thread { public void run() { // we should not get past this lock // since it is held by the main thread synchronized(lock) { System.out.println("inside"); } } } public static void main(String... args) throws Exception { synchronized(lock) { Thread t = new MyThread(); t.start(); t.join(); } } // called from JVMTI agent static void callback() { synchronized(lock) { <== The assert happens here at the MonitorEnter bytecode System.out.println("callback"); } } } The agent MonitorContendedEnter callback: void JNICALL myMonitorContendedEnter(jvmtiEnv *jvmti_env, JNIEnv* jni_env, jthread thread, jobject object) { char* name; jclass c; c = (*jni_env)->GetObjectClass(jni_env, object); (*jvmti_env)->GetClassSignature(jvmti_env, c, &name, NULL); printf("myMonitorContendedEnter %s\n", name); // call back out to Java and try to take this same lock jmethodID mid = (*jni_env)->GetStaticMethodID(jni_env, c, "callback", "()V"); (*jni_env)->CallStaticVoidMethod(jni_env, c, mid); <== Here the method "Recursive.callback() is called } As we can see above the agent callback does an unsafe JNI call to the callback() method which cause a recursive lock of the monitor. This potentially can lead to the endless recursion of the MyMonitorContendedEnter() callback calls and so is UNSAFE. I'm closing this bug as "Not an issue".
03-10-2014
Attached hs_err_pid43821.log: crash from running netbeans profiler and running into this problem (from the original report in JDK-8035150).
27-02-2014
The description states "and from that callback tries to take the same lock again", but this can happen if it tries to acquire any lock.
27-02-2014
This is not about recursive acquisition of monitors, it is about the reentrancy of the code used to implement monitors and mutexes - that code is NOT designed to be reentrant.
24-02-2014
I'm wondering if the only real problem in this case is the assert itself: it is ok to come back here recursively from the JVMTI hook but the assert does not take that path into account.
21-02-2014
Coleen: "ObjectMonitors are reentrant, just not Mutexes."
21-02-2014
As noted in other JVMTI bug reports the JVMTI spec is somewhat lacking in restricting callbacks from performing unsafe actions. Many of the events for which callbacks can be set are in code that is itself not reentrant - eg sync code. So you could argue that the callback can only be reentrant if everything it calls is also reentrant and hence this is an error in the callback. But of course the callback writer has no way to know what events involve non-reentrant VM code.
13-02-2014
I temporarily assigned this bug to myself to check what is needed here.
13-02-2014
There are some general guidelines in the JVMTI spec about handling events: http://docs.oracle.com/javase/7/docs/platform/jvmti/jvmti.html#EventSection Specifically, it tells this: "Event callback functions must be re-entrant. The JVM TI implementation does not queue events. If an agent needs to process events one at a time, it can use a raw monitor inside the event callback functions to serialize event processing." The specific scenario from the bug description looks like a non-re-entrant handling of the monitor events. But it requires more analysis to find out what exact actions must be taken, and if there are some ways in agents to avoid such issues.
13-02-2014
Attaching a reproducer. To build: $ gcc -shared -o librec.dylib -I $JAVA_HOME/include/ -I $JAVA_HOME/include/darwin/ rec.c $ javac Recursive.java To run: $ java -agentlib:rec Recursive
12-02-2014