The test failure is caused by the arrival of unexpected ThreadStartEvents, 
which mess up the debugger side. The events are for threads we normally only 
see getting created when using virtual threads, such as carrier threads and the 
VirtualThread-unparker thread. Theoretically this issue could happen without 
virtual threads due to other VM threads starting up such as Common-Cleaner, but 
we haven't seen it fail for that reason yet.

The test is testing proper thread suspension for ThreadStartEvent using each of 
the 3 suspension policy types. The debuggee creates a sequence of 3 debuggee 
threads, each one's timing coordinated with some complicated synchronization 
with the debugger using breakpoints and the setting of fields in the debuggee 
(and careful placement of suspend/resume in the debugger). The 
ThreadStartRequests that the debugger sets up always use a "count filter" of 1, 
which means the requests are good for delivering exactly 1 ThreadStartEvent, 
and any that come after the first will get filtered out. So when an an 
unexpected ThreadStartEvent arrives for something like a carrier thread, this 
prematurely moves the debugger on to the next step, and the synchronization 
with the debuggee gets messed up.

The first step in fixing this test was to remove the count filter, so the 
request can handle any number of ThreadStartEvents.

The next step was then fixing the test library code in EventHandler.java so it 
would filter out any undesired ThreadStartEvents, so the test just ends up 
getting one event, and always for the thread it is expecting. There are a few 
parts to this. One is improving EventFilters.filter() to filter out more 
threads that tend to be created during VM startup, including carrier threads 
and the VirtualThread-unparker thread.

It was necessary to add some calls EventFilters.filter() from EventHandler. 
This was done by adding a ThreadStartEvent listener for the "spurious" thread 
starts (those the test debuggee does not create). This listener is added by 
waitForRequestedEventCommon(), which is indirectly called by the test when is 
calls waitForRequestedEventSet().

There is a also 2nd place where the ThreadStartEvent listener for "spurious" 
threads is needed. It is now also installed with the default listeners that are 
always in place. It is needed when the test is not actually waiting for a 
ThreadStartEvent, but is waiting for a BreakpointEvent. 
waitForRequestedEventCommon() is not used in this case (so none of its 
listeners are installed), but the default listeners are always in place and can 
be used to filter these ThreadStartEvents. Note this filter will also be in 
place when calling waitForRequestedEventCommon(), but we can't realy on it when 
waitForRequestedEventCommon() is used for ThreadStartEvents because the 
spurious ThreadStartEvent will be seen and returned before we ever get to the 
default filter. So we actually end up with this ThreadStartEvent listener 
installed twice during  waitForRequestedEventCommon().

I did a bit of cleanup on the test, mostly renaming of threads and 
ThreadStartRequests so they are easier to match up with the iteration # we use 
in both the debuggee and debugger (0, 1, and 2). The only real change in the 
test itself is removing the filter count, and verifying that the 
ThreadStartEvent is for the expected thread.

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

Commit messages:
 - Filter out spurious ThreadStartEvents.

Changes: https://git.openjdk.org/jdk/pull/12861/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12861&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8289765
  Stats: 96 lines in 4 files changed: 73 ins; 1 del; 22 mod
  Patch: https://git.openjdk.org/jdk/pull/12861.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12861/head:pull/12861

PR: https://git.openjdk.org/jdk/pull/12861

Reply via email to