On Fri, 3 Mar 2023 18:16:25 GMT, Chris Plummer <[email protected]> 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.
Marked as reviewed by sspitsyn (Reviewer).
Looks good.
Thanks,
Serguei
test/hotspot/jtreg/vmTestbase/nsk/share/jdi/EventHandler.java line 270:
> 268: if (event instanceof ThreadStartEvent) {
> 269: if (EventFilters.filtered(event)) {
> 270: display(owner +": Ignoring spurious thread
> creation: " + event);
Nit: Space is needed after first '+' sign.
-------------
PR: https://git.openjdk.org/jdk/pull/12861Marked as reviewed by sspitsyn
(Reviewer).
