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

[Daniel D. Daugherty]

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

Thumbs up. I have some questions, but I don't think anything is blocking.

src/hotspot/share/runtime/synchronizer.cpp line 1094:

> 1092: // Iterate owned ObjectMonitors.
> 1093: void ObjectSynchronizer::owned_monitors_iterate(MonitorClosure* closure) {
> 1094:   auto all_filter = [&](void* owner) { return true; };

I don't grok how this filter only iterates owned ObjectMonitors.
It always returns `true` and does not check for a non-null owner.
I'm probably missing some sort of lambda magic here...

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

> 342:   void do_monitor(ObjectMonitor* monitor) override {
> 343:     // The caller is interested in the owned ObjectMonitors. This does
> 344:     // not include when owner is set to a stack-lock address in thread.

The stack-lock part of this comment doesn't agree with
the header comment for this relocated version of the code.

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

> 390: 
> 391:     // If there are many object monitors in the system then the above iteration
> 392:     // can start to to take time. Be friendly to following thread dumps by

nit typo: s/start to to take/start to take/

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

> 67: 
> 68:     static private void createMonitors() {
> 69:         Object[] monitors = new Object[1000];

Since `monitors` is local to this static function, if the test runs for long
enough, then C2 might optimize away all those monitors...

I usually ask @vnkozlov about the best way to keep that from happening.

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

Marked as reviewed by dcubed (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16519#pullrequestreview-1721324825
PR Review Comment: https://git.openjdk.org/jdk/pull/16519#discussion_r1387230578
PR Review Comment: https://git.openjdk.org/jdk/pull/16519#discussion_r1387238228
PR Review Comment: https://git.openjdk.org/jdk/pull/16519#discussion_r1387239156
PR Review Comment: https://git.openjdk.org/jdk/pull/16519#discussion_r1387248957