On Tue, 25 Nov 2025 19:13:44 GMT, Chris Plummer <[email protected]> wrote:

>> Actually it appears that JDWP THREAD_START can have a filter but JVMTI 
>> THREAD_START does not allow you to set the thread to enable the events on. 
>> That means when JDWP THREAD_START events are requested and a thread filter 
>> is provided, JVMTI THREAD_START needs to be enabled for all threads. That's 
>> actually how it is working now. I tried adding the assert and it was 
>> triggered. So that means passing "thread" instead of NULL would sometimes 
>> end up passing an actual thread to JVMTI and it would produce a 
>> [JVMTI_ERROR_ILLEGAL_ARGUMENT in that case.
>
> There  is a bug in the call to eventHandlerRestricted_iterator() at line 
> 1436. For VIRTUAL_THREAD_START I should be explicitly passing in NULL for the 
> thread. Otherwise it will consider the thread when deciding if other event 
> handlers are a match to this one, and I don't think we want that. It could 
> lead to disabling VIRTUAL_THREAD_START events when we shouldn't. Seems we 
> don't have any tests for this but I modified an existing test and got a 
> failure. The test creates a ThreadStartRequest with a thread filter and 
> expects that to trigger a ThreadStartEvent for the filter thread. If I create 
> a second ThreadStartRequest with a filter on a different thread, enable it, 
> and then disable it, that ends up disabling all VIRTUAL_THREAD_START events, 
> and the expected event never arrives. Passing NULL fixes the problem.

Ok, the last problem I pointed out is fixed now. The fix was done in 
matchHasNoPlatformThreadsOnlyFilter(). I was going to pass NULL for "thread" 
when ei == EI_THREAD_START, but this turned out not to be enough and changes 
were also needed in matchHasNoPlatformThreadsOnlyFilter(). Once those changes 
were in place there was no longer a need to pass NULL for "thread".

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28485#discussion_r2561360150

Reply via email to