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

[Chris Plummer]

On Wed, 26 Mar 2025 16:39:27 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Regarding adding a comment, I wasn't too sure what the comment should say because once you start down that path, it's hard to not end up with too much detail, and then things just get very wordy. Since we have a test that will fail if this is ever modified to cause classloading again, at least that should help prevent undoing this fix. However, the one downside of the test is that I can only get it to fail with non-debug builds, so it may not be caught early.
>> 
>> Regarding using the loop, that's probably a more bullet proof option, although there is still no guarantee that someday someone won't look at it and say "I can just use toArray() here". There's probably no escaping an AI code cleaning bot coming to that conclusion sometime in the near future. Then we're back to adding a comment to make sure it isn't changed.
>
> I just noticed that this `subgroupsAsArray()` private method is only used by JVMTI and a comment exists about that on that method:
> 
> 
> /**
>  * Returns a snapshot of the subgroups as an array, used by JVMTI.
>  */
> 
> So, irrespective of what solution we choose, perhaps we could add another brief sentence to that comment, something like the following?
> 
> 
> /**
>  * Returns a snapshot of the subgroups as an array, used by JVMTI.
>  * Care should be taken not to trigger any classloading in this method.
>  */

I switched to the code that does the array copy inline rather than rely on toArray(). I also added a comment. I intentionally kept is short and simple. I was tempted to say something about toArray(), reference the CR, mention JDI and the debug agent, etc, but that's heading down the "too wordy" path that I wanted to avoid. Readers can "git blame" to find out why it was done.

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

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