RFR: 8352088: Call of com.sun.jdi.ThreadReference.threadGroups() can lock up target VM [v2]

[Alan Bateman]

On Wed, 26 Mar 2025 20:37:46 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> Yes, one EventHandler thread dispatching to all listeners. More details below:
>> 
>> There is an interface called TargetListener which declares all the event callbacks. TargetAdapter implements TargetListener and provides and empty implementation for each callback. TestScaffold extends TargetAdapter, and each test extends TestScaffold. So every test has default empty implementations of all the callbacks, and can override some (there are a couple of overrides in this tests).
>> 
>> A TargetListener instance gets events by adding itself as a listener using TestScaffold.addListener(). You can see that this test is doing that early on so it gets events.
>> 
>> In addition, some of the TestScaffold APIs will create an instance of a TargetAdapter with one or more event callback overrides, and register that listener. For example, see waitForRequestedEvent(). So it is possible to have 2 or more listeners registered, although as I just pointed out in [JDK-8352759](https://bugs.openjdk.org/browse/JDK-8352759), that can be problematic at times.
>> 
>> TestScaffold always has an EventHandler thread active (and only one). It sits in a loop calling EventQueue.remove(). For any each EventSet received it will iterate over each Event in the EventSet, and for each registered listener, it will call the listener's event callback for that Event.
>
> BTW, classPreparedCount is somewhat unneeded legacy code. Previously I had limited it to 50 events to avoid the VMDisconnected exception, but I got rid of the count check and instead now catch the exception.

Thanks for the expanded comment, it just means that classPreparedCount doesn't need to be volatile, it's only accessed by one thread. For this test it doesn't matter of course.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24236#discussion_r2015857410