On Wed, 8 Mar 2023 22:39:55 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. > > Chris Plummer has updated the pull request incrementally with one additional > commit since the last revision: > > Fix a couple of typos Thanks Kevin and Serguei! ------------- PR: https://git.openjdk.org/jdk/pull/12861
