On Thu, 11 Jul 2024 22:36:05 GMT, Chris Plummer <[email protected]> wrote:
>> The test hits a breakpoint on thread2 with SUSPEND_EVENT_THREAD policy, so
>> only thread2 is suspended. It then does a vm.suspend(), which suspends all
>> threads and bumps the suspendCount of thread2 up to 2. It then does an
>> eventSet.resume(), which decrements the thread2 suspendCount to 1, so now
>> all threads are suspended with a suspendCount of 1. thread2 is then resumed
>> and we expect to hit the next thread2 breakpoint. The problem is that
>> thread2 can't hit the breakpoint until the main thread has proceeded far
>> enough, and if the vm.suspend() that suspended the main thread happens too
>> quickly, it won't have proceeded far enough, so thread2 never hits the
>> breakpoint.
>>
>> Essentially we need a vm.resume() to allow the main thread to run, but we
>> need to do it in a way that does nullify part of what the test is testing
>> for. So in order to allow the vm.resume() but not subvert the intent of the
>> test, we first do a thread2.suspend() so the vm.resume() won't resume
>> thread2.
>>
>> Testing in progress: tier1 and tier5 svc testing
>
> Chris Plummer has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix copyright and jcheck error
FWIW I think the explicit sync with the mainthread seems reasonable too.
test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/resume/resume001.java
line 382:
> 380: if (expresult != returnCode0) {
> 381: vm.resume();
> 382: vm.resume(); // for case error when both VirtualMachine
> and the thread2 were suspended
Pre-existing but I don't understand the comment. Why would you need 2
`vm.resume()` here? If `thread2` was suspended directly don't you need a
`thread2.resume()`?
-------------
PR Review: https://git.openjdk.org/jdk/pull/20088#pullrequestreview-2173568846
PR Review Comment: https://git.openjdk.org/jdk/pull/20088#discussion_r1675036756
