On Fri, 3 Mar 2023 18:16:25 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
> 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. This pull request has now been integrated. Changeset: 8b0eb729 Author: Chris Plummer <cjplum...@openjdk.org> URL: https://git.openjdk.org/jdk/commit/8b0eb7299a5d0e142453ed5c7a17308077e27993 Stats: 97 lines in 4 files changed: 73 ins; 1 del; 23 mod 8289765: JDI EventSet/resume/resume008 failed with "ERROR: suspendCounts don't match for : VirtualThread-unparker" Reviewed-by: sspitsyn, kevinw ------------- PR: https://git.openjdk.org/jdk/pull/12861
