[GR-60633] Fix JDWP reconnect logic not resetting all state properly.

PullRequest: graal/19704

Author: javeleon

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

@@ -114,4 +114,6 @@ public interface FieldRef {
      * @return true if this field has any breakpoints, false otherwise
      */
     boolean hasActiveBreakpoint();
+
+    void disposeFieldBreakpoint();
 }

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

@@ -29,6 +29,7 @@
 import com.oracle.truffle.api.frame.Frame;
 import com.oracle.truffle.api.nodes.Node;
 import com.oracle.truffle.api.nodes.RootNode;
+import com.oracle.truffle.espresso.jdwp.impl.DebuggerController;
 
 /**
  * Interface that defines required methods for a guest language when implementing JDWP.
@@ -500,4 +501,6 @@ public interface JDWPContext {
     Object allocateInstance(KlassRef klass);
 
     void steppingInProgress(Thread t, boolean value);
+
+    void replaceController(DebuggerController newController);
 }

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

@@ -232,6 +232,8 @@ public interface MethodRef {
      */
     boolean hasActiveHook();
 
+    void disposeHooks();
+
     /**
      * Determine if this method is obsolete. A method is obsolete if it has been replaced by a
      * non-equivalent method using the RedefineClasses command. The original and redefined methods

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

@@ -90,6 +90,10 @@ public void activate(Object mainThread, DebuggerController control, JDWPContext
         this.ids = context.getIds();
     }
 
+    public void replaceController(DebuggerController newController) {
+        this.debuggerController = newController;
+    }
+
     @Override
     public void onDetach() {
         // free up request, to avoid attempting to send anything further

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

@@ -58,4 +58,8 @@ public interface BreakpointInfo {
     void addSuspendPolicy(byte suspendPolicy);
 
     byte getSuspendPolicy();
+
+    default void dispose() {
+        // do nothing by default
+    }
 }

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

@@ -157,6 +157,7 @@ public void reInitialize() {
         DebuggerController newController = new DebuggerController(jdwpLogger);
         newController.truffleContext = truffleContext;
         newController.initialize(debugger, options, context, initialThread, eventListener);
+        context.replaceController(newController);
         assert newController.setupState != null;
 
         if (newController.setupState.fatalConnectionError) {
@@ -186,6 +187,10 @@ public void reset(boolean prepareForReconnect) {
             // existence state.
             resetting.lockInterruptibly();
 
+            // end the current debugger session to avoid hitting any further breakpoints
+            // when resuming all threads
+            endSession();
+
             currentReceiverThread = receiverThread;
 
             // Close the server socket used to listen for transport dt_socket.
@@ -207,16 +212,14 @@ public void reset(boolean prepareForReconnect) {
             // re-enable GC for all objects
             getGCPrevention().clearAll();
 
-            // end the current debugger session to avoid hitting any further breakpoints
-            // when resuming all threads
-            endSession();
-
-            // resume all threads
-            forceResumeAll();
+            eventFilters.clearAll();
 
             // Now, close the socket, which will force the receiver thread to complete eventually.
             // Note that we might run this code in the receiver thread, so we can't simply join.
             closeSocket();
+
+            // resume all threads
+            forceResumeAll();
         } catch (InterruptedException e) {
             Thread.currentThread().interrupt();
         } finally {
@@ -822,8 +825,8 @@ private void lockThread(Object thread, boolean forceSuspend, List<Callable<Void>
         while (!Thread.currentThread().isInterrupted()) {
             try {
                 synchronized (lock) {
-                    if (!lock.isLocked()) {
-                        // released from other thread, so break loop
+                    if (!lock.isLocked() || !connection.isOpen()) {
+                        // released from other thread or session ended, so break loop
                         break;
                     }
                     // no reason to hold a hard suspension status, since now

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

@@ -62,4 +62,20 @@ public RequestFilter getRequestFilter(int requestId) {
             readLock.unlock();
         }
     }
+
+    public void clearAll() {
+        try {
+            lock.writeLock().lock();
+            // traverse all filters and clear all registered breakpoint information
+            for (RequestFilter requestFilter : requestFilters) {
+                BreakpointInfo breakpointInfo = requestFilter.getBreakpointInfo();
+                if (breakpointInfo != null) {
+                    breakpointInfo.dispose();
+                }
+            }
+            requestFilters = new RequestFilter[0];
+        } finally {
+            lock.writeLock().unlock();
+        }
+    }
 }

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

@@ -63,4 +63,9 @@ public boolean isModificationBreakpoint() {
     public boolean isAccessBreakpoint() {
         return accessBreakpoint;
     }
+
+    @Override
+    public void dispose() {
+        field.disposeFieldBreakpoint();
+    }
 }

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

@@ -63,4 +63,11 @@ public boolean onMethodEnter(@SuppressWarnings("unused") MethodRef method, @Supp
     public boolean onMethodExit(@SuppressWarnings("unused") MethodRef method, @SuppressWarnings("unused") Object returnValue) {
         return isMethodExit;
     }
+
+    @Override
+    public void dispose() {
+        for (MethodRef method : methods) {
+            method.disposeHooks();
+        }
+    }
 }

espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/impl/Field.java

@@ -945,11 +945,16 @@ public final void removeFieldBreakpointInfo(int requestId) {
                     // remove index 1, but keep info at index 0
                     temp[0] = infos[0];
                     infos = temp;
-                    return;
                 }
         }
     }
 
+    @Override
+    public void disposeFieldBreakpoint() {
+        hasActiveBreakpoints.set(false);
+        infos = null;
+    }
+
     public void setCompatibleField(@SuppressWarnings("unused") Field field) {
         // only applicable to RedefineAddedFields
     }

espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/impl/Method.java

@@ -1120,6 +1120,11 @@ public synchronized void removeActiveHook(MethodHook hook) {
         }
     }
 
+    public synchronized void disposeHooks() {
+        hasActiveHook.set(false);
+        hooks = MethodHook.EMPTY;
+    }
+
     public SharedRedefinitionContent redefine(ObjectKlass.KlassVersion klassVersion, ParserMethod newMethod, ParserKlass newKlass, Ids<Object> ids) {
         // install the new method version immediately
         LinkedMethod newLinkedMethod = new LinkedMethod(newMethod);
@@ -1890,6 +1895,11 @@ public void removedMethodHook(MethodHook hook) {
             getMethod().removeActiveHook(hook);
         }
 
+        @Override
+        public void disposeHooks() {
+            getMethod().disposeHooks();
+        }
+
         @Override
         public boolean hasActiveHook() {
             return getMethod().hasActiveHook();

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

@@ -98,18 +98,20 @@ public final class JDWPContextImpl implements JDWPContext {
     private RedefinitionPluginHandler redefinitionPluginHandler;
     private final ArrayList<ReloadingAction> classInitializerActions = new ArrayList<>(1);
     private DebuggerController controller;
+    private VMEventListenerImpl vmEventListener;
 
     public JDWPContextImpl(EspressoContext context) {
         this.context = context;
         this.ids = new Ids<>(StaticObject.NULL);
         this.innerClassRedefiner = new InnerClassRedefiner(context);
     }
 
-    public void jdwpInit(TruffleLanguage.Env env, Object mainThread, VMEventListenerImpl vmEventListener) {
+    public void jdwpInit(TruffleLanguage.Env env, Object mainThread, VMEventListenerImpl eventListener) {
         Debugger debugger = env.lookup(env.getInstruments().get("debugger"), Debugger.class);
         this.controller = env.lookup(env.getInstruments().get(JDWPInstrument.ID), DebuggerController.class);
-        vmEventListener.activate(mainThread, controller, this);
-        controller.initialize(debugger, context.getEspressoEnv().JDWPOptions, this, mainThread, vmEventListener);
+        vmEventListener = eventListener;
+        eventListener.activate(mainThread, controller, this);
+        controller.initialize(debugger, context.getEspressoEnv().JDWPOptions, this, mainThread, eventListener);
         redefinitionPluginHandler = RedefinitionPluginHandler.create(context);
         classRedefinition = context.createClassRedefinition(ids, redefinitionPluginHandler);
     }
@@ -120,6 +122,12 @@ public void finalizeContext() {
         }
     }
 
+    @Override
+    public void replaceController(DebuggerController newController) {
+        this.controller = newController;
+        vmEventListener.replaceController(newController);
+    }
+
     @Override
     public Ids<Object> getIds() {
         return ids;