RFR: 8296089: Remove debug agent code for special handling of Thread.resume() [v2]

[Serguei Spitsyn]

On Wed, 9 Nov 2022 05:57:32 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> The debug agent sets a breakpoint in Thread.resume() so it can prevent the debugger from suspending threads while in the resume call:
>> 
>>              /*
>>               * Track the resuming thread by marking it as being within
>>               * a resume and by setting up for notification on
>>               * a frame pop or exception. We won't allow the debugger
>>               * to suspend threads while any thread is within a
>>               * call to resume. This (along with the block below)
>>               * ensures that when the debugger
>>               * suspends a thread it will remain suspended.
>>               */
>>              trackAppResume(resumer);
>> 
>> Now that Thread.resume() is unsupported and just throws UnsupportedOperationException, all debug agent code related to this support can be removed. It's at least a couple of hundred lines of code, and with a fair amount of confusing synchronization. It will be nice to see it go.
>
> Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Cleanup some suspect suspendOnStart and toBeResumed checks.

src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 737:

> 735:         node->toBeResumed = JNI_TRUE;
> 736:     }
> 737:     JDI_ASSERT(error != JVMTI_ERROR_THREAD_SUSPENDED);

It'd be nice to add a small comment explaining why this asserted is here.

src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 841:

> 839:         debugMonitorNotifyAll(threadLock);
> 840:         if ((node->suspendCount == 0) && node->toBeResumed) {
> 841:             JDI_ASSERT(!node->suspendOnStart);

It'd be nice to add a small comment explaining why this asserted is here.

src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 949:

> 947:      */
> 948:     if (node->suspendCount == 1 && node->suspendOnStart) {
> 949:         JDI_ASSERT(!node->toBeResumed);

It'd be nice to add a small comment explaining why this asserted is here.

src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 966:

> 964:      */
> 965:     if (node->suspendCount == 1 && node->toBeResumed) {
> 966:         JDI_ASSERT(!node->suspendOnStart);

It'd be nice to add a small comment explaining why this asserted is here.

src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 989:

> 987:      */
> 988:     if (node->suspendCount == 1 && node->toBeResumed) {
> 989:         JDI_ASSERT(!node->suspendOnStart);

It'd be nice to add a small comment explaining why this asserted is here.

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

PR: https://git.openjdk.org/jdk/pull/10922