Consolidate logic in DebuggerController and reduce dependencies of the controller. Always create a new DebuggerController to avoid multiple controllers operating on the same underlying socket

Author: javeleon

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

@@ -22,6 +22,7 @@
  */
 package com.oracle.truffle.espresso.jdwp.api;
 
+import com.oracle.truffle.api.TruffleLogger;
 import com.oracle.truffle.api.debug.DebugScope;
 import com.oracle.truffle.api.debug.DebugStackFrame;
 import com.oracle.truffle.api.debug.DebugValue;
@@ -34,10 +35,8 @@
 import com.oracle.truffle.api.interop.UnsupportedMessageException;
 import com.oracle.truffle.api.nodes.Node;
 import com.oracle.truffle.api.nodes.RootNode;
-import com.oracle.truffle.espresso.jdwp.impl.DebuggerController;
 
 public final class CallFrame {
-
     private static final InteropLibrary INTEROP = InteropLibrary.getFactory().getUncached();
     public static final Object INVALID_VALUE = new Object();
 
@@ -53,11 +52,11 @@ public final class CallFrame {
     private final DebugStackFrame debugStackFrame;
     private final DebugScope debugScope;
     private final JDWPContext context;
-    private final DebuggerController controller;
     private Object scope;
+    private final TruffleLogger logger;
 
     public CallFrame(long threadId, byte typeTag, long classId, MethodRef method, long methodId, long codeIndex, Frame frame, Node currentNode, RootNode rootNode,
-                    DebugStackFrame debugStackFrame, JDWPContext context, DebuggerController controller) {
+                    DebugStackFrame debugStackFrame, JDWPContext context, TruffleLogger logger) {
         this.threadId = threadId;
         this.typeTag = typeTag;
         this.classId = classId;
@@ -70,11 +69,12 @@ public CallFrame(long threadId, byte typeTag, long classId, MethodRef method, lo
         this.debugStackFrame = debugStackFrame;
         this.debugScope = debugStackFrame != null ? debugStackFrame.getScope() : null;
         this.context = context;
-        this.controller = controller;
+        this.logger = logger;
     }
 
-    public CallFrame(long threadId, byte typeTag, long classId, long methodId, long codeIndex) {
-        this(threadId, typeTag, classId, null, methodId, codeIndex, null, null, null, null, null, null);
+    // used for tests in comparisons only
+    public CallFrame(long threadId, byte typeTag, long classId, long methodId, long codeIndex, TruffleLogger logger) {
+        this(threadId, typeTag, classId, null, methodId, codeIndex, null, null, null, null, null, logger);
     }
 
     public byte getTypeTag() {
@@ -124,9 +124,7 @@ public Object getThisValue() {
             }
             return INTEROP.readMember(theScope, "0");
         } catch (UnsupportedMessageException | UnknownIdentifierException e) {
-            if (controller != null) {
-                controller.warning(() -> "Unable to read 'this' value from method: " + getMethod() + " with currentNode: " + currentNode.getClass());
-            }
+            logger.warning(() -> "Unable to read 'this' value from method: " + getMethod() + " with currentNode: " + currentNode.getClass());
             return INVALID_VALUE;
         }
     }
@@ -144,9 +142,7 @@ public void setVariable(Object value, String identifier) {
         try {
             INTEROP.writeMember(theScope, identifier, value);
         } catch (Exception e) {
-            if (controller != null) {
-                controller.warning(() -> "Unable to write member " + identifier + " from variables");
-            }
+            logger.warning(() -> "Unable to write member " + identifier + " from variables");
         }
     }
 
@@ -164,14 +160,10 @@ private Object getScope() {
             try {
                 scope = NodeLibrary.getUncached().getScope(node, frame, true);
             } catch (UnsupportedMessageException e) {
-                if (controller != null) {
-                    controller.warning(() -> "Unable to get scope for " + currentNode.getClass());
-                }
+                logger.warning(() -> "Unable to get scope for " + currentNode.getClass());
             }
         } else {
-            if (controller != null) {
-                controller.warning(() -> "Unable to get scope for " + currentNode.getClass());
-            }
+            logger.warning(() -> "Unable to get scope for " + currentNode.getClass());
         }
         return scope;
     }

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

@@ -31,7 +31,7 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import com.oracle.truffle.espresso.jdwp.impl.DebuggerController;
+import com.oracle.truffle.api.TruffleLogger;
 
 /**
  * Class that keeps an ID representation of all entities when communicating with a debugger through
@@ -57,11 +57,10 @@ public final class Ids<T> {
 
     private HashMap<String, Long> innerClassIDMap = new HashMap<>(16);
 
-    private DebuggerController controller;
-
     private List<Object> pinnedObjects = new ArrayList<>();
 
     private volatile boolean pinningState = false;
+    private TruffleLogger logger;
 
     @SuppressWarnings({"unchecked", "rawtypes"})
     public Ids(T nullObject) {
@@ -202,14 +201,8 @@ public boolean checkRemoved(long refTypeId) {
         return innerClassIDMap.containsValue(refTypeId);
     }
 
-    public void injectController(DebuggerController control) {
-        this.controller = control;
-    }
-
     private void log(Supplier<String> supplier) {
-        if (controller != null) {
-            controller.finest(supplier);
-        }
+        logger.finest(supplier);
     }
 
     public synchronized void pinAll() {
@@ -226,4 +219,8 @@ public synchronized void unpinAll() {
         pinningState = false;
         pinnedObjects.clear();
     }
+
+    public void injectLogger(TruffleLogger truffleLogger) {
+        this.logger = truffleLogger;
+    }
 }

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

@@ -201,7 +201,7 @@ public void run() {
             }
             // Now, begin processing packets when they start to flow from the debugger
             try {
-                while (!Thread.currentThread().isInterrupted()) {
+                while (!Thread.currentThread().isInterrupted() && !controller.isClosing()) {
                     try {
                         processPacket(Packet.fromByteArray(debuggerConnection.connection.readPacket()));
                     } catch (IOException e) {
@@ -242,7 +242,7 @@ private void processPacket(Packet packet) {
                                     result = JDWP.VirtualMachine.CLASSES_BY_SIGNATURE.createReply(packet, controller, context);
                                     break;
                                 case JDWP.VirtualMachine.ALL_CLASSES.ID:
-                                    result = JDWP.VirtualMachine.ALL_CLASSES.createReply(packet, context);
+                                    result = JDWP.VirtualMachine.ALL_CLASSES.createReply(packet, context, controller);
                                     break;
                                 case JDWP.VirtualMachine.ALL_THREADS.ID:
                                     result = JDWP.VirtualMachine.ALL_THREADS.createReply(packet, context, controller);