RFR: 8072701: resume001 failed due to ERROR: timeout for waiting for a BreakpintEvent [v3]

[Chris Plummer]

On Fri, 12 Jul 2024 08:02:31 GMT, Alex Menkov <amenkov at openjdk.org> wrote:

>> First just to clarify a general JDI feature about thread suspending and resuming. You can undo a ThreadReference.suspend() or a thread suspended as the result of an event by dong a vm.resume(). This is documented in the JDI API spec, which talks about suspendCounts and how various APIs and event delivery affect them.
>> 
>> I was  tempted to clean up these vm.resume() calls but opted not to. The point being made in the comment is that worse case thread2 was suspended by a breakpoint or thread2.suspend() and all threads were suspended by a vm.resume() (meaning thread2 could have a suspendCount of 2). Two vm.resumes() take are done to make sure thread2 gets resumed under this situation. However, one of the vm.resume calls could instead be a thread2.resume(). Doing two vm.resume() calls was probably just laziness by the original  authors. It works though.
>> 
>> However, by my accounting at any failure point thread2 never has a suspendCount > 1, so really just one vm.resume() would be enough.
>> 
>> The original code did these two vm.resume() calls unconditionally, but they are not needed if there was no error.
>
> The original code had 2 vm.resume() - one on them to match vm.suspend() and 2nd one to allow debugee to continue on error.
> Now we have 3 vm.resume() - one is to match vm.suspend() (line 377) and 2 conditional (on error).
> Theoretically we can get an error when both vm and thread2 are suspended, so 2 vm.resume() looks reasonable.
> Anyway resume() is a nop if the thread is not suspended

After reaching the 2nd breakpoint, which suspends thread2, we do a vm.suspend(), which bumps the thread2 suspendCount to 2. However, we do a eventSet.resume() after this, lowering the suspendCount to 1, and there is no error bailout point while the suspendCount is 2. Thus only 1 vm.resume() is needed in the error handling.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20088#discussion_r1676208721