RFR: 8318757: VM_ThreadDump asserts in interleaved ObjectMonitor::deflate_monitor calls [v5]

[David Holmes]

On Wed, 8 Nov 2023 13:34:24 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> A safepointed monitor deflation pass can run interleaved with a paused async monitor deflation pass. The code is not written to handle that situation and asserts when it finds a DEFLATER_MARKER in the owner field. @pchilano also found other issues with having to monitor deflation passes interleaved. More info below ...
>
> Stefan Karlsson has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Rename monitors_iterate

Just a couple of drive by comments. For something that was summarized so succinctly( "fix for this is to stop deflating monitors from the safepoting VM_ThreadDump operation") the actual changes are quite extensive and I found it difficult to follow.

But thanks for the fix!

src/hotspot/share/runtime/synchronizer.hpp line 135:

> 133: 
> 134:   // Iterate owned ObjectMonitors.
> 135:   static void owned_monitors_iterate(MonitorClosure* closure);

owned by whom? current thread? Does that include stack-locked or not?

Just trying to understand how the two variants of `owned_monitors_iterate` relate.

src/hotspot/share/runtime/vmOperations.cpp line 395:

> 393:     // telling the MonitorDeflationThread to deflate monitors.
> 394:     //
> 395:     // The limit has been arbitrarily chosen to be were the iteration started

s/were/where/

test/hotspot/jtreg/runtime/Monitor/ConcurrentDeflation.java line 78:

> 76:             monitors[index] = new Object();
> 77:             synchronized (monitors[index]) {
> 78:             }

I would expect C2 to eliminate this as well. The monitors are provably thread-local so synchronization is a no-op.

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

PR Review: https://git.openjdk.org/jdk/pull/16519#pullrequestreview-1721529428
PR Review Comment: https://git.openjdk.org/jdk/pull/16519#discussion_r1387370859
PR Review Comment: https://git.openjdk.org/jdk/pull/16519#discussion_r1387372489
PR Review Comment: https://git.openjdk.org/jdk/pull/16519#discussion_r1387377594