RFR: 8320515: assert(monitor->object_peek() != nullptr) failed: Owned monitors should not have a dead object [v4]

[David Holmes]

On Thu, 23 Nov 2023 08:40:55 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> src/hotspot/share/runtime/vmOperations.cpp line 354:
>> 
>>> 352:       // alive. Filter out monitors with dead objects.
>>> 353:       return;
>>> 354:     }
>> 
>> I don't think we need to do this, but even without this filtering I ran a number of tests and was unable to demonstrate any problem. The JNI locked monitor seems to be "invisible" to the frame that locked it and so the thread dump never encounters it. Were you able to provoke a failure here or is this defensive programming?
>
> I provoked test failures for all paths I filtered. If I remove this check and run:
> 
> make -C ../build/fastdebug test TEST=test/hotspot/jtreg/runtime/Monitor/IterateMonitorWithDeadObjectTest.java JTREG="JAVA_OPTIONS=-XX:+UseZGC"
> 
> 
> I hit this assert:
> 
> #  Internal Error (/home/stefank/git/jdk/open/src/hotspot/share/services/management.cpp:1274), pid=1546709, tid=1546754
> #  assert(object != nullptr) failed: must be a Java object
> ...
> V  [libjvm.so+0x1330ce8]  jmm_DumpThreads+0x1a48  (management.cpp:1274)
> j  sun.management.ThreadImpl.dumpThreads0([JZZI)[Ljava/lang/management/ThreadInfo;+0 java.management at 22-internal
> j  sun.management.ThreadImpl.dumpAllThreads(ZZI)[Ljava/lang/management/ThreadInfo;+28 java.management at 22-internal
> j  sun.management.ThreadImpl.dumpAllThreads(ZZ)[Ljava/lang/management/ThreadInfo;+5 java.management at 22-internal
> j  IterateMonitorWithDeadObjectTest.dumpThreadsWithLockedMonitors()V+7
> j  IterateMonitorWithDeadObjectTest.main([Ljava/lang/String;)V+11
> 
> 
> If I remove that assert I hit an NPE in the Java layer:
> 
> java.lang.NullPointerException: Cannot invoke "Object.getClass()" because "lock" is null
> 	at java.management/java.lang.management.ThreadInfo.<init>(ThreadInfo.java:172)
> 	at java.management/sun.management.ThreadImpl.dumpThreads0(Native Method)
> 	at java.management/sun.management.ThreadImpl.dumpAllThreads(ThreadImpl.java:518)
> 	at java.management/sun.management.ThreadImpl.dumpAllThreads(ThreadImpl.java:506)
> 	at IterateMonitorWithDeadObjectTest.dumpThreadsWithLockedMonitors(IterateMonitorWithDeadObjectTest.java:44)
> 	at IterateMonitorWithDeadObjectTest.main(IterateMonitorWithDeadObjectTest.java:66)
> 	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
> 	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
> 	at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:333)
> 	at java.base/java.lang.Thread.run(Thread.java:1570)

Thanks for that. Looks like JMM thread dump is different to VM Thread dump. Okay we definitely need RFEs to look into how to handle this.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16783#discussion_r1403992144