[GR-59465] Fix disabling debugger stepping too aggressively.

PullRequest: graal/19238

Author: javeleon

espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/JDWPContext.java

@@ -498,4 +498,6 @@ public interface JDWPContext {
     boolean isSingleSteppingDisabled();
 
     Object allocateInstance(KlassRef klass);
+
+    void steppingInProgress(Thread t, boolean value);
 }

espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerController.java

@@ -204,6 +204,7 @@ public void setCommandRequestId(Object thread, int commandRequestId, byte suspen
         fine(() -> "Adding step command request in thread " + getThreadName(thread) + " with ID: " + commandRequestId);
         SteppingInfo steppingInfo = new SteppingInfo(commandRequestId, suspendPolicy, isPopFrames, isForceEarlyReturn, stepKind);
         commandRequestIds.put(thread, steppingInfo);
+        context.steppingInProgress(getContext().asHostThread(thread), true);
     }
 
     /**
@@ -825,6 +826,7 @@ public void onSuspend(SuspendedEvent event) {
             if (steppingInfo != null) {
                 if (steppingInfo.isForceEarlyReturn()) {
                     fine(() -> "not suspending here due to force early return: " + event.getSourceSection());
+                    context.steppingInProgress(hostThread, false);
                     return;
                 }
                 CallFrame[] callFrames = createCallFrames(ids.getIdAsLong(currentThread), event.getStackFrames(), 1, steppingInfo);
@@ -860,14 +862,21 @@ public void onSuspend(SuspendedEvent event) {
                     });
                 } else {
                     fine(() -> "step not completed - check for breakpoints");
-                    continueStepping(event, steppingInfo, currentThread);
-                    commandRequestIds.put(currentThread, steppingInfo);
 
                     result = checkForBreakpoints(event, jobs, currentThread, callFrames);
+                    if (!result.breakpointHit) {
+                        // no breakpoint
+                        continueStepping(event, steppingInfo, currentThread);
+                        commandRequestIds.put(currentThread, steppingInfo);
+                    }
                 }
             } else {
                 result = checkForBreakpoints(event, jobs, currentThread, callFrames);
             }
+            if (!commandRequestIds.containsKey(currentThread)) {
+                // we're done stepping then
+                context.steppingInProgress(hostThread, false);
+            }
             if (result.skipSuspend) {
                 return;
             }

espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/EspressoLanguage.java

@@ -686,13 +686,17 @@ public DisableSingleStepping disableStepping() {
 
     public final class DisableSingleStepping implements AutoCloseable {
 
+        private final boolean steppingDisabled;
+
         private DisableSingleStepping() {
-            getThreadLocalState().disableSingleStepping();
+            steppingDisabled = getThreadLocalState().disableSingleStepping(false);
         }
 
         @Override
         public void close() {
-            getThreadLocalState().enableSingleStepping();
+            if (steppingDisabled) {
+                getThreadLocalState().enableSingleStepping();
+            }
         }
     }
 

espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/EspressoThreadLocalState.java

@@ -43,6 +43,8 @@ public class EspressoThreadLocalState {
 
     private int singleSteppingDisabledCounter;
 
+    private boolean stepInProgress;
+
     @SuppressWarnings("unused")
     public EspressoThreadLocalState(EspressoContext context) {
         typeStack = new ClassRegistry.TypeStack();
@@ -124,8 +126,16 @@ public VM.PrivilegedStack getPrivilegedStack() {
         return privilegedStack;
     }
 
-    public void disableSingleStepping() {
-        singleSteppingDisabledCounter++;
+    public void setSteppingInProgress(boolean value) {
+        stepInProgress = value;
+    }
+
+    public boolean disableSingleStepping(boolean forceDisable) {
+        if (forceDisable || stepInProgress) {
+            singleSteppingDisabledCounter++;
+            return true;
+        }
+        return false;
     }
 
     public void enableSingleStepping() {

espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/JDWPContextImpl.java

@@ -276,6 +276,17 @@ public Object allocateInstance(KlassRef klass) {
         return context.getAllocator().createNew((ObjectKlass) klass);
     }
 
+    @Override
+    public void steppingInProgress(Thread t, boolean value) {
+        Object previous = null;
+        try {
+            previous = controller.enterTruffleContext();
+            context.getLanguage().getThreadLocalStateFor(t).setSteppingInProgress(value);
+        } finally {
+            controller.leaveTruffleContext(previous);
+        }
+    }
+
     @Override
     public Object[] getAllGuestThreads() {
         StaticObject[] activeThreads = context.getActiveThreads();

espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/substitutions/Target_org_graalvm_continuations_ContinuationImpl.java

@@ -58,7 +58,7 @@ static void suspend0(StaticObject self, @Inject EspressoLanguage language, @Inje
         // reflection or create the FrameRecords in the guest.
         // Re-enabled in the catch clause of Continuation.resume0().
         // TODO(GR-54326): Let Truffle Instrumentation know.
-        tls.disableSingleStepping();
+        tls.disableSingleStepping(true);
 
         // This internal exception will be caught in BytecodeNode's interpreter loop. Frame records
         // will be added to the exception object in a linked list until it's caught in resume below.
@@ -96,7 +96,7 @@ static boolean resume0(StaticObject self,
                 // Disable stepping until we have fully re-winded.
                 // Re-enabled in ResumeNextContinuationNode.dolast()
                 // TODO(GR-54326): Let Truffle Instrumentation know.
-                tls.disableSingleStepping();
+                tls.disableSingleStepping(true);
                 rewind.execute(stack);
                 // Normal completion.
                 return false;