On Tue, 25 Nov 2025 17:33:04 GMT, Chris Plummer <[email protected]> wrote:

>> THREAD_START can't have a thread filter. THREAD_END can. "thread" == 
>> requestThread(node) == the thread filter if there is one, so when disabling 
>> THREAD_START it could be an actual thread or it could just be NULL. For 
>> THREAD_START "thread" should always be NULL, so I suppose I could have just 
>> asserted that and used "thread" as the argument instead of "NULL".
>
> 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.

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

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

Reply via email to