On Wed, 10 Jul 2024 20:56:17 GMT, Alex Menkov <amen...@openjdk.org> wrote:

>> I was able to add synchronization between the debugger and debuggee to fix 
>> this issue, but I don't really like the solution. It just adds more 
>> complexity and makes the test even harder to follow. What I'd like to do is 
>> just put a short sleep in before the vm.suspend(). Let me know if you think 
>> this is ok and I'll update the PR with the changes.
>
> Thank you for the confirming the reason of the timeout.
> 
> To be more clear about my point:
> The test has 3 scenarios (see the test description):
> ThreadReference.resume() resumes the thread suspended with:
>  *   - with thread2.suspend()                                           <BR>
>  *   - at a breakpoint                                                  <BR>
>  *   - with VirtualMachine.suspend()                                    <BR>
> 
> So for 3rd scenario we should not call vm.resume() (as it converts 3rd 
> scenario to 1st scenario).
> The test can be fixed by different ways, to me remove logging between 
> breakpoint2 and breakpoint3 is the simplest way.
> Note that breakpoint2 is "runt2(), line 2" and breakpoint3 is "runt1(), line 
> 7", there are 2 log statements. We can move breakpoint 3 to "runt2(), line 3" 
> (I don't see much sense to have breakpoint 3 so far from breakpoint2 - we 
> just need to ensure the thread was resumed )

Removing log() statements to fix the problem can be risky because someone could 
re-add them in the future. What about my idea of doing a short sleep before the 
vm.suspend() to make sure the main thread has advanced to the pipe.readln(), 
and won't be doing any more log calls until it gets the next command from the 
debugger (which should be "quit").

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

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

Reply via email to