On Fri, 12 Jul 2024 08:02:31 GMT, Alex Menkov <amen...@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
