From 7e113676fac3f17b774a444ad07a3d3050b4816d Mon Sep 17 00:00:00 2001 From: "Alexey.Merkulov" Date: Mon, 6 Jan 2025 20:43:45 +0100 Subject: [PATCH] [debugger] Remove the stepping request in the current thread if a coroutine suspends This commit fixes stepping in runBlocking, IDEA-369686. And also it introduces the regression in the stepOverWithContext test. But the next commit will fix it. GitOrigin-RevId: 76750510f23d37501dee0d8ff073adcbbf792f67 --- .../debugger/engine/DebugProcessEvents.java | 6 +- .../stepping/CoroutineBreakpointFacility.kt | 84 ++++++++++++++++++- .../stepOver/soSuspendableCallInEndOfFun.out | 2 +- 3 files changed, 86 insertions(+), 6 deletions(-) diff --git a/java/debugger/impl/src/com/intellij/debugger/engine/DebugProcessEvents.java b/java/debugger/impl/src/com/intellij/debugger/engine/DebugProcessEvents.java index 85a5a3fa9151..ec53b876d1d4 100644 --- a/java/debugger/impl/src/com/intellij/debugger/engine/DebugProcessEvents.java +++ b/java/debugger/impl/src/com/intellij/debugger/engine/DebugProcessEvents.java @@ -551,7 +551,7 @@ public class DebugProcessEvents extends DebugProcessImpl { RequestHint hint = getRequestHint(event); Object commandToken = getCommandToken(event); - deleteStepRequests(suspendContext.getVirtualMachineProxy().eventRequestManager(), event.thread()); + removeStepRequests(suspendContext, thread); boolean shouldResume = false; @@ -611,6 +611,10 @@ public class DebugProcessEvents extends DebugProcessImpl { } } + public static void removeStepRequests(@NotNull SuspendContextImpl suspendContext, @Nullable ThreadReference thread) { + suspendContext.getDebugProcess().deleteStepRequests(suspendContext.getVirtualMachineProxy().eventRequestManager(), thread); + } + // Preload event info in "parallel" commands, to avoid sync jdwp requests after static void preloadEventInfo(ThreadReference thread, @Nullable Location location) { if (Registry.is("debugger.preload.event.info") && DebuggerUtilsAsync.isAsyncEnabled()) { diff --git a/plugins/kotlin/jvm-debugger/core/src/org/jetbrains/kotlin/idea/debugger/core/stepping/CoroutineBreakpointFacility.kt b/plugins/kotlin/jvm-debugger/core/src/org/jetbrains/kotlin/idea/debugger/core/stepping/CoroutineBreakpointFacility.kt index 156e76e54e55..8e1abc11b62b 100644 --- a/plugins/kotlin/jvm-debugger/core/src/org/jetbrains/kotlin/idea/debugger/core/stepping/CoroutineBreakpointFacility.kt +++ b/plugins/kotlin/jvm-debugger/core/src/org/jetbrains/kotlin/idea/debugger/core/stepping/CoroutineBreakpointFacility.kt @@ -3,14 +3,17 @@ package org.jetbrains.kotlin.idea.debugger.core.stepping import com.intellij.debugger.DebuggerManagerEx -import com.intellij.debugger.engine.DebugProcessImpl -import com.intellij.debugger.engine.StepIntoMethodBreakpoint -import com.intellij.debugger.engine.SuspendContextImpl +import com.intellij.debugger.engine.* import com.intellij.debugger.engine.events.SuspendContextCommandImpl +import com.intellij.debugger.settings.DebuggerSettings +import com.intellij.debugger.ui.breakpoints.SteppingBreakpoint +import com.intellij.debugger.ui.breakpoints.SyntheticLineBreakpoint import com.intellij.openapi.diagnostic.debug import com.intellij.openapi.diagnostic.thisLogger +import com.intellij.openapi.project.Project import com.intellij.openapi.util.registry.Registry import com.sun.jdi.Location +import com.sun.jdi.ThreadReference import com.sun.jdi.event.LocatableEvent import org.jetbrains.kotlin.idea.debugger.base.util.safeMethod import org.jetbrains.kotlin.idea.debugger.core.StackFrameInterceptor @@ -49,7 +52,11 @@ object CoroutineBreakpointFacility { private fun installCoroutineResumedBreakpoint(context: SuspendContextImpl, resumedLocation: Location, nextLocationAfterResume: Location?): Boolean { val debugProcess = context.debugProcess - debugProcess.cancelRunToCursorBreakpoint() + debugProcess.cancelSteppingBreakpoints() + val clearSteppingBreakpoint = installBreakpointToRemoveSteppingInCurrentThread(context) + if (clearSteppingBreakpoint == null) { + thisLogger().warn("No clear stepping breakpoint installed for context $context") + } val project = debugProcess.project val useCoroutineIdFiltering = Registry.`is`("debugger.filter.breakpoints.by.coroutine.id") @@ -75,6 +82,13 @@ object CoroutineBreakpointFacility { if (!result) return false val suspendContextImpl = action.suspendContext ?: return true + clearSteppingBreakpoint?.let { + if (!it.steppingRemoved) { + thisLogger().debug("Clear old stepping from resume breakpoint") + it.removeRequestAndStepping(suspendContextImpl) + } + } + return scheduleStepOverCommandForSuspendSwitch(suspendContextImpl, nextLocationAfterResume) } @@ -98,6 +112,68 @@ object CoroutineBreakpointFacility { return true } + + private fun installBreakpointToRemoveSteppingInCurrentThread(context: SuspendContextImpl): ClearSteppingBreakpoint? { + val classLoader = context.frameProxy?.classLoader ?: return null + val originalThread = context.thread?.threadReference ?: return null + + val debugProbesImpl = + context.debugProcess.findLoadedClass(context, "kotlinx.coroutines.debug.internal.DebugProbesImpl", classLoader) ?: return null + + val methods = debugProbesImpl.methods() ?: return null + + val probeSuspendedMethod = methods.singleOrNull { it.name().contains("probeCoroutineSuspended") } ?: return null + + val locationForBP = probeSuspendedMethod.locationOfCodeIndex(0) + + val breakpoint = ClearSteppingBreakpoint(context.debugProcess.project, originalThread) + + breakpoint.suspendPolicy = DebuggerSettings.SUSPEND_THREAD + + val requestsManager = context.debugProcess.requestsManager + val request = requestsManager.createBreakpointRequest(breakpoint, locationForBP) + request.addThreadFilter(originalThread) + requestsManager.enableRequest(request) + context.debugProcess.setSteppingBreakpoint(breakpoint) + return breakpoint + } +} + +private class ClearSteppingBreakpoint(project: Project, private val originalThread: ThreadReference) : SyntheticLineBreakpoint(project), SteppingBreakpoint { + var steppingRemoved = false + private set + + override fun shouldIgnoreThreadFiltering() = true + + override fun track() = false + + override fun processLocatableEvent(action: SuspendContextCommandImpl, event: LocatableEvent?): Boolean { + val suspendContext = action.suspendContext + val currentThread = suspendContext?.thread?.threadReference + if (originalThread == currentThread) { + removeRequestAndStepping(suspendContext) + } else { + // This should not happen because of the filter on the thread for this request + thisLogger().error("Skip remove stepping breakpoint for thread ${originalThread.name()}") + } + return false + } + + fun removeRequestAndStepping(suspendContext: SuspendContextImpl) { + if (steppingRemoved) { + return + } + steppingRemoved = true + thisLogger().debug { "Remove stepping requests $suspendContext" } + DebugProcessEvents.removeStepRequests(suspendContext, originalThread) + suspendContext.debugProcess.requestsManager.deleteRequest(this) + } + + override fun isRestoreBreakpoints() = false + + override fun setRequestHint(hint: RequestHint) { + error("Should not be called") + } } fun SuspendContextImpl.getLocationCompat(): Location? { diff --git a/plugins/kotlin/jvm-debugger/test/testData/evaluation/singleBreakpoint/coroutines/stepOver/soSuspendableCallInEndOfFun.out b/plugins/kotlin/jvm-debugger/test/testData/evaluation/singleBreakpoint/coroutines/stepOver/soSuspendableCallInEndOfFun.out index f6fd13395357..ad4b435dd664 100644 --- a/plugins/kotlin/jvm-debugger/test/testData/evaluation/singleBreakpoint/coroutines/stepOver/soSuspendableCallInEndOfFun.out +++ b/plugins/kotlin/jvm-debugger/test/testData/evaluation/singleBreakpoint/coroutines/stepOver/soSuspendableCallInEndOfFun.out @@ -2,9 +2,9 @@ LineBreakpoint created at soSuspendableCallInEndOfFun.kt:20 Run Java Connected to the target VM soSuspendableCallInEndOfFun.kt:20 -soSuspendableCallInEndOfFun.kt:10 soSuspendableCallInEndOfFun.kt:11 soSuspendableCallInEndOfFun.kt:8 +soSuspendableCallInEndOfFun.kt:13 Disconnected from the target VM Process finished with exit code 0 \ No newline at end of file