[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8296089: Remove debug agent code for special handling of Thread.resume()
From:       Chris Plummer <cjplummer () openjdk ! org>
Date:       2022-10-31 19:21:08
Message-ID: e9M8xz3XaTKb07ihduPv4ug9_15HVk9_MEqAV7v8eu0=.6b452ec6-7749-45b8-bb23-b97453f29323 () github ! com
[Download RAW message or body]

On Mon, 31 Oct 2022 19:08:14 GMT, Chris Plummer <cjplummer@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.  
> 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


[prev in list] [next in list] [prev in thread] [next in thread]