On Tue, 22 Sep 2026 06:44:57 GMT, David Holmes <[email protected]> wrote:
>> test/jdk/com/sun/management/HotSpotDiagnosticMXBean/DumpThreads.java line
>> 347:
>>
>>> 345: assertNotNull(fields, "thread not found");
>>> 346: assertEquals("WAITING", fields.state());
>>> 347: assertFalse(contains(dumpForThread(tid, lines), "-
>>> locked <" + lockAsString));
>>
>> The plain text format thread is not specified. It's too fragile to be be
>> trying to extract a "section". I think dumpForThread should be removed, a
>> simple one line for search for the identity string of the lock object is
>> sufficient here. The testing of the JSON format, just below, will do the
>> right check of the owned monitors.
>
> Thank for re-examining @AlanBateman .
>
> We search for the thread section in `findThread` which relies on the `# tid`
> logic, so I was simply re-applying that to extract the section for that
> thread. Granted it does assume `#` is only used to start a new thread
> section, but if we changed that the test would start to fail and we could
> update the logic.
>
> The simple one line search does not work because now we own the monitor in
> the main thread so that "locked" line appears in the dump. The whole point is
> to check that such a "locked" line does not appear for the thread doing the
> wait. The original logic sufficed when only the waiter potentially owned the
> lock during the dump.
>
> The main thread has to take the lock to ensure the target has fully released
> it, otherwise it could report locked or not depending on when the dump
> request struck.
My strong preference is to not attempt to parse sections of the plain text
thread dump. It's unstructured and not intended to be parsed like this. I would
prefer to keep this type of testing to the JSON thread dump because it is
structured and much more reliable to test.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32977#discussion_r4069108539
