On Mon, 31 Oct 2022 19:08:14 GMT, Chris Plummer <[email protected]> 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.
>
> Also, there is a little bit of code to deal with the app calling
> Thread.suspend(). I'm a bit unsure of it's removal, because I'm not certain
> if we also need to consider another JVMTI agent doing a suspend. However, we
> don't seem to defend against another JVMTI agent resuming a thread, so maybe
> the debug agent is just not expected to work if another JVMTI agent
> interferes. I would appreciate some insight on this possibility. I've called
> out this code in the inlined review comments.
src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 741:
> 739: node->toBeResumed = JNI_TRUE;
> 740: }
> 741: JDI_ASSERT(error != JVMTI_ERROR_THREAD_SUSPENDED);
Should the original code remain in place because another JVMTI agent may have
suspended the thread?
src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 952:
> 950: * won't suspend this thread after we are done with the resumeAll.
> 951: */
> 952: if (node->suspendCount == 1 && node->suspendOnStart) {
This is also related to Thread.suspend() and the previous comment I made above.
-------------
PR: https://git.openjdk.org/jdk/pull/10922
