On Tue, 5 Oct 2021 05:31:37 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
> I'm not so sure this is always safe. It might be fine in the context of > resetting the connection, but not during normal debug agent operations. It > allows for another event to be processed when the lock is suppose to keep > event processing serialized. What happens for example if we hit the > `Thread.resume()` breakpoint again on a different thread? I agree that this is probably not safe. E.g. when releasing handlerLock the handler chain for EI_BREAKPOINT could be modified which is being iterated in the caller function event_callback(). https://github.com/openjdk/jdk/blob/8609ea55acdcc203408f58f7bf96ea9228aef613/src/jdk.jdwp.agent/share/native/libjdwp/eventHandler.c#L647-L673 I should have checked before. > It might be best to limit doing this only when `threadControl_reset()` is > currently executing (you could set a flag there), although it seems more like > a band aid than a proper fix. I could imagine there might still be scenarios > were releasing the lock during reset might be problematic, although probably > extremely unlikely to ever be noticed. I looked at threadControl_resumeThread(). It appears to me that resuming a thread is not possible while some thread is waiting in blockOnDebuggerSuspend() because threadControl_resumeThread() locks handlerLock. https://github.com/openjdk/jdk/blob/32811026ce5ecb1d27d835eac33de9ccbd51fcbf/src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c#L1485 Am I missing something? Maybe the code in handleAppResumeBreakpoint() could moved to doPendingTasks()? ------------- PR: https://git.openjdk.java.net/jdk/pull/5805
