On Wed, 26 Mar 2025 18:22:00 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> Calling ThreadGroupReference.groups() from an event handler can cause a
>> deadlock. Details in first comment. Tested with :jdk_lang on all supported
>> platforms and tier1, tier2, tier3, and tier5 svc testing.
>
> Chris Plummer has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Use explicit array copying code. Better comment.
Marked as reviewed by alanb (Reviewer).
src/java.base/share/classes/java/lang/ThreadGroup.java line 661:
> 659: /**
> 660: * Returns a snapshot of the subgroups as an array, used by JVMTI.
> 661: * WARNING: Make sure this API does not trigger any class loading.
Might be better to say "method" rather than "API" as this is all internal.
test/jdk/com/sun/jdi/EarlyThreadGroupChildrenTest.java line 55:
> 53: public class EarlyThreadGroupChildrenTest extends TestScaffold {
> 54: ClassType targetClass;
> 55: ThreadReference mainThread;
Are these used? Maybe copied over from another test?
test/jdk/com/sun/jdi/EarlyThreadGroupChildrenTest.java line 83:
> 81: public void classPrepared(ClassPrepareEvent event) {
> 82: try {
> 83: ++classPreparedCount;
It's a long standing since I looked at at test based on TestScaffold. Would I
be correct to say that it creates an event handler thread to remove events
from the event queue and dispatch them to the registered listeners? Only asking
as it's not immediately clear if the increment of the event count is correct. I
think TestScaffold uses one thread but would need to dig into the test infra to
be sure.
-------------
PR Review: https://git.openjdk.org/jdk/pull/24236#pullrequestreview-2718340853
PR Review Comment: https://git.openjdk.org/jdk/pull/24236#discussion_r2014828734
PR Review Comment: https://git.openjdk.org/jdk/pull/24236#discussion_r2014831539
PR Review Comment: https://git.openjdk.org/jdk/pull/24236#discussion_r2014843667
