On Thu, 21 Apr 2022 20:14:49 GMT, Chris Plummer <[email protected]> wrote:
> The test is testing that EventSets for ThreadStartEvents have the proper
> suspendPolicy. When there is more than one ThreadStartRequest and a thread is
> started, each ThreadStartRequest results in a ThreadStartEvent being created,
> and they all are grouped into the same EventSet. The suspendPolicy on this
> EventSet should come from the ThreadStartRequest suspend policy that suspends
> the most threads. The test is testing for all possible combinations of the 3
> suspend polices (NONE, THREAD, and ALL). For example, if NONE and ALL are
> used, the resulting suspend policy should be ALL.
>
> The following 3 issues are being addressed. These all turned up in loom as a
> result of carrier threads being created while the test was running, but
> potentially could happen with other threads that the jvm starts up.
>
> 1. When the test calls getEventSet() for the next ThreadStartEvent, it
> sometimes gets one that is for a carrier thread. In general this is not a
> problem because the test will accept any thread, but sometimes this carrier
> thread is generated between setting up two different `ThreadStartRequests`,
> and as a result has the wrong suspend policy, so the test fails with:
>
> nsk.share.TestFailure: ##> debugger: ERROR: eventSet.suspendPolicy() !=
> policyExpected
>
> This is fixed by using getEventSetForThreadStartDeath(<threadName>) instead
> of getEventSet(), so carrier threads (and any other spuriously created
> thread) can be ignored.
>
> 2. The ThreadStartRequests used a filterCount of 1, which meant they would
> only produce one ThreadStartEvent. This meant that after fix (1) was in
> place, I started seeing issues with not even seeing the expected
> ThreadStartEvent, because the 1 count was used up by the carrier thread
> starting. I don't see any reason for this filterCount (other than the issue
> described in 3), so I just removed it.
>
> 3. After (1) and (2) were in place, I then noticed issues with sometimes
> getting a ThreadStartEvent when the BreakpointEvent was expected. This is
> because sometime a carrier thread was being created after receiving the
> expected ThreadStartEvent, but before the ThreadStartRequests could be
> disabled (this is the result of getting rid of the counterFiler). This was
> fixed by having breakpointForCommunication() filter out unexpected
> ThreadStartEvents by calling EventFilters.filtered(). I also had to add
> carrier threads that EventFilters.filtered() filters out.
It looks good.
I've only requested to add a simple comment.
Thanks,
Serguei
test/hotspot/jtreg/vmTestbase/nsk/share/jdi/EventFilters.java line 342:
> 340: if (event.toString().contains("ForkJoinPool"))
> 341: return true;
> 342:
As I understand, this is to filter out events for carrier threads.
Is it right? If so, it'd be good to add a short comment to make it clear.
-------------
Marked as reviewed by sspitsyn (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/8350
