> 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 ------------- Changes: - all: https://git.openjdk.org/jdk/pull/12861/files - new: https://git.openjdk.org/jdk/pull/12861/files/f159416c..ea759bb3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=12861&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=12861&range=00-01 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 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
