Support attach debug & remove restartRequest support (eclipse-jdtls#42)

* Support attach debug & remove restartRequest support Signed-off-by: Jinbo Wang <[email protected]> * fix review comments * Use extension factory to implement singleton pattern * Stop all debug connections when stopping JavaDebugServer
fbricon · Aug 17, 2017 · 6e52f64 · 6e52f64
1 parent 57325af
commit 6e52f64
Show file tree

Hide file tree

Showing 6 changed files with 168 additions and 78 deletions.
diff --git a/org.eclipse.jdt.ls.debug/plugin.xml b/org.eclipse.jdt.ls.debug/plugin.xml
@@ -8,7 +8,7 @@
          id="org.eclipse.jdt.ls.debug.javadebugger"
          point="org.eclipse.jdt.ls.core.debugserver">
       <implementor
-            class="org.eclipse.jdt.ls.debug.adapter.jdt.JavaDebugServer"
+            class="org.eclipse.jdt.ls.debug.adapter.jdt.DebugServerFactory"
             type="java">
       </implementor>
    </extension>

diff --git a/org.eclipse.jdt.ls.debug/src/org/eclipse/jdt/ls/debug/DebugUtility.java b/org.eclipse.jdt.ls.debug/src/org/eclipse/jdt/ls/debug/DebugUtility.java
@@ -22,6 +22,7 @@
 import com.sun.jdi.Location;
 import com.sun.jdi.ThreadReference;
 import com.sun.jdi.VirtualMachineManager;
+import com.sun.jdi.connect.AttachingConnector;
 import com.sun.jdi.connect.Connector.Argument;
 import com.sun.jdi.connect.IllegalConnectorArgumentsException;
 import com.sun.jdi.connect.LaunchingConnector;
@@ -67,8 +68,31 @@ public static IDebugSession launch(VirtualMachineManager vmManager, String mainC
         return new DebugSession(connector.launch(arguments));
     }
 
-    public static IDebugSession attach(/* TODO: arguments? */) {
-        throw new UnsupportedOperationException();
+    /**
+     * Attach to an existing debuggee VM.
+     * @param vmManager
+     *               the virtual machine manager
+     * @param hostName
+     *               the machine where the debuggee VM is launched on
+     * @param port
+     *               the debug port that the debuggee VM exposed
+     * @param attachTimeout
+     *               the timeout when attaching to the debuggee VM
+     * @return an instance of IDebugSession
+     * @throws IOException
+     *               when unable to attach.
+     * @throws IllegalConnectorArgumentsException
+     *               when one of the connector arguments is invalid.
+     */
+    public static IDebugSession attach(VirtualMachineManager vmManager, String hostName, int port, int attachTimeout)
+            throws IOException, IllegalConnectorArgumentsException {
+        List<AttachingConnector> connectors = vmManager.attachingConnectors();
+        AttachingConnector connector = connectors.get(0);
+        Map<String, Argument> arguments = connector.defaultArguments();
+        arguments.get("hostname").setValue(hostName);
+        arguments.get("port").setValue(String.valueOf(port));
+        arguments.get("timeout").setValue(String.valueOf(attachTimeout));
+        return new DebugSession(connector.attach(arguments));
     }
 
     /**

diff --git a/org.eclipse.jdt.ls.debug/src/org/eclipse/jdt/ls/debug/adapter/DebugAdapter.java b/org.eclipse.jdt.ls.debug/src/org/eclipse/jdt/ls/debug/adapter/DebugAdapter.java
@@ -60,7 +60,8 @@ public class DebugAdapter implements IDebugAdapter {
     private boolean clientLinesStartAt1 = true;
     private boolean clientPathsAreUri = false;
 
-    private Requests.LaunchArguments launchArguments;
+    private boolean isAttached = false;
+
     private String cwd;
     private String[] sourcePath;
     private IDebugSession debugSession;
@@ -101,10 +102,6 @@ public Messages.Response dispatchRequest(Messages.Request request) {
                     responseBody = attach(JsonUtils.fromJson(arguments, Requests.AttachArguments.class));
                     break;
 
-                case "restart":
-                    responseBody = restart(JsonUtils.fromJson(arguments, Requests.RestartArguments.class));
-                    break;
-
                 case "disconnect":
                     responseBody = disconnect(JsonUtils.fromJson(arguments, Requests.DisconnectArguments.class));
                     break;
@@ -254,50 +251,41 @@ private Responses.ResponseBody initialize(Requests.InitializeArguments arguments
         Types.Capabilities caps = new Types.Capabilities();
         caps.supportsConfigurationDoneRequest = true;
         caps.supportsHitConditionalBreakpoints = true;
-        caps.supportsRestartRequest = true;
         caps.supportTerminateDebuggee = true;
         return new Responses.InitializeResponseBody(caps);
     }
 
     private Responses.ResponseBody launch(Requests.LaunchArguments arguments) {
-        // Need cache the launch json because VSCode doesn't resend the launch json at the RestartRequest.
-        this.launchArguments = arguments;
         try {
+            this.isAttached = false;
             this.launchDebugSession(arguments);
         } catch (DebugException e) {
+            // When launching failed, send a TerminatedEvent to tell DA the debugger would exit.
+            this.sendEventLater(new Events.TerminatedEvent());
             return new Responses.ErrorResponseBody(
                     this.convertDebuggerMessageToClient("Cannot launch debuggee vm: " + e.getMessage()));
         }
         return new Responses.ResponseBody();
     }
 
     private Responses.ResponseBody attach(Requests.AttachArguments arguments) {
-        return new Responses.ResponseBody();
-    }
-
-    private Responses.ResponseBody restart(Requests.RestartArguments arguments) {
-        // Shutdown the old debug session.
-        this.shutdownDebugSession(true);
-        // Launch new debug session.
         try {
-            this.launchDebugSession(this.launchArguments);
+            this.isAttached = true;
+            this.attachDebugSession(arguments);
         } catch (DebugException e) {
+            // When attaching failed, send a TerminatedEvent to tell DA the debugger would exit.
+            this.sendEventLater(new Events.TerminatedEvent());
             return new Responses.ErrorResponseBody(
-                    this.convertDebuggerMessageToClient("Cannot restart debuggee vm: " + e.getMessage()));
+                    this.convertDebuggerMessageToClient(e.getMessage()));
         }
-        // See VSCode bug 28175 (https://github.com/Microsoft/vscode/issues/28175).
-        // Need send a ContinuedEvent to clean up the old debugger's call stacks.
-        this.sendEventLater(new Events.ContinuedEvent(true));
-        // Send an InitializedEvent to ask VSCode to restore the existing breakpoints.
-        this.sendEventLater(new Events.InitializedEvent());
         return new Responses.ResponseBody();
     }
 
     /**
      * VS Code terminates a debug session with the disconnect request.
      */
     private Responses.ResponseBody disconnect(Requests.DisconnectArguments arguments) {
-        this.shutdownDebugSession(arguments.terminateDebuggee);
+        this.shutdownDebugSession(arguments.terminateDebuggee && !this.isAttached);
         return new Responses.ResponseBody();
     }
 
@@ -587,16 +575,30 @@ private void launchDebugSession(Requests.LaunchArguments arguments) throws Debug
         }
     }
 
+    private void attachDebugSession(Requests.AttachArguments arguments) throws DebugException {
+        this.cwd = arguments.cwd;
+        if (arguments.sourcePath == null || arguments.sourcePath.length == 0) {
+            this.sourcePath = new String[] { cwd };
+        } else {
+            this.sourcePath = new String[arguments.sourcePath.length];
+            System.arraycopy(arguments.sourcePath, 0, this.sourcePath, 0, arguments.sourcePath.length);
+        }
+
+        try {
+            this.debugSession = DebugUtility.attach(context.getVirtualMachineManagerProvider().getVirtualMachineManager(),
+                    arguments.hostName, arguments.port, arguments.attachTimeout);
+        } catch (IOException | IllegalConnectorArgumentsException e) {
+            Logger.logException("Failed to attach to remote debuggee vm. Reason: " + e.getMessage(), e);
+            throw new DebugException("Failed to attach to remote debuggee vm. Reason: " + e.getMessage(), e);
+        }
+    }
+
     private void shutdownDebugSession(boolean terminateDebuggee) {
-        // Unsubscribe event handler.
-        this.eventSubscriptions.forEach(subscription -> {
-            subscription.dispose();
-        });
         this.eventSubscriptions.clear();
         this.breakpointManager.reset();
         this.frameCollection.reset();
         this.sourceCollection.reset();
-        if (this.debugSession.process().isAlive()) {
+        if (this.debugSession != null) {
             if (terminateDebuggee) {
                 this.debugSession.terminate();
             } else {

diff --git a/org.eclipse.jdt.ls.debug/src/org/eclipse/jdt/ls/debug/adapter/Requests.java b/org.eclipse.jdt.ls.debug/src/org/eclipse/jdt/ls/debug/adapter/Requests.java
@@ -45,7 +45,15 @@ public static class LaunchArguments extends Arguments {
     }
 
     public static class AttachArguments extends Arguments {
-
+        public String type;
+        public String name;
+        public String request;
+        public String cwd;
+        public String hostName;
+        public int port;
+        public int attachTimeout;
+        public String[] sourcePath = new String[0];
+        public String projectName;
     }
 
     public static class RestartArguments extends Arguments {

diff --git a/org.eclipse.jdt.ls.debug/src/org/eclipse/jdt/ls/debug/adapter/jdt/DebugServerFactory.java b/org.eclipse.jdt.ls.debug/src/org/eclipse/jdt/ls/debug/adapter/jdt/DebugServerFactory.java
@@ -0,0 +1,24 @@
+/*******************************************************************************
+* Copyright (c) 2017 Microsoft Corporation and others.
+* All rights reserved. This program and the accompanying materials
+* are made available under the terms of the Eclipse Public License v1.0
+* which accompanies this distribution, and is available at
+* http://www.eclipse.org/legal/epl-v10.html
+*
+* Contributors:
+*     Microsoft Corporation - initial API and implementation
+*******************************************************************************/
+
+package org.eclipse.jdt.ls.debug.adapter.jdt;
+
+import org.eclipse.core.runtime.CoreException;
+import org.eclipse.core.runtime.IExecutableExtensionFactory;
+
+public class DebugServerFactory implements IExecutableExtensionFactory {
+
+    @Override
+    public Object create() throws CoreException {
+        return JavaDebugServer.getInstance();
+    }
+
+}
diff --git a/org.eclipse.jdt.ls.debug/src/org/eclipse/jdt/ls/debug/adapter/jdt/JavaDebugServer.java b/org.eclipse.jdt.ls.debug/src/org/eclipse/jdt/ls/debug/adapter/jdt/JavaDebugServer.java
@@ -17,80 +17,94 @@
 import java.io.PrintWriter;
 import java.net.ServerSocket;
 import java.net.Socket;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.SynchronousQueue;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jdt.ls.core.debug.IDebugServer;
 import org.eclipse.jdt.ls.debug.adapter.ProtocolServer;
 import org.eclipse.jdt.ls.debug.internal.Logger;
 
 public class JavaDebugServer implements IDebugServer {
+    private static JavaDebugServer singletonInstance;
+
     private ServerSocket serverSocket = null;
-    private Socket connection = null;
-    private ProtocolServer protocolServer = null;
+    private boolean isStarted = false;
+    private ExecutorService executor = null;
 
-    /**
-     * Constructs a JavaDebugServer instance which will launch a ServerSocket to 
-     * listen for incoming socket connection.
-     */
-    public JavaDebugServer() {
+    private JavaDebugServer() {
         try {
             this.serverSocket = new ServerSocket(0, 1);
         } catch (IOException e) {
-            Logger.logException("Create ServerSocket exception", e);
+            Logger.logException("Failed to create Java Debug Server", e);
         }
     }
 
-    @Override
-    public int getPort() {
+    /**
+     * Gets the single instance of JavaDebugServer.
+     * @return the JavaDebugServer instance
+     */
+    public static synchronized IDebugServer getInstance() {
+        if (singletonInstance == null) {
+            singletonInstance = new JavaDebugServer();
+        }
+        return singletonInstance;
+    }
+
+    /**
+     * Gets the server port.
+     */
+    public synchronized int getPort() {
         if (this.serverSocket != null) {
             return this.serverSocket.getLocalPort();
         }
         return -1;
     }
 
-    @Override
-    public void start() {
-        if (this.serverSocket != null) {
+    /**
+     * Starts the server if it's not started yet.
+     */
+    public synchronized void start() {
+        if (this.serverSocket != null && !this.isStarted) {
+            this.isStarted = true;
+            this.executor = new ThreadPoolExecutor(0, 100, 30L, TimeUnit.SECONDS, new SynchronousQueue<Runnable>());
             // Execute eventLoop in a new thread.
             new Thread(new Runnable() {
 
                 @Override
                 public void run() {
-                    int serverPort = -1;
-                    try {
-                        // It's blocking here to waiting for incoming socket connection.
-                        connection = serverSocket.accept();
-                        serverPort = serverSocket.getLocalPort();
-                        closeServerSocket(); // Stop listening for further connections.
-                        Logger.logInfo("Start debugserver on socket port " + serverPort);
-                        BufferedReader in = new BufferedReader(new InputStreamReader(connection.getInputStream()));
-                        PrintWriter out = new PrintWriter(connection.getOutputStream(), true);
-
-                        protocolServer = new ProtocolServer(in, out, JdtProviderContextFactory.createProviderContext());
-                        // protocol server will dispatch request and send response in a while-loop.
-                        protocolServer.start();
-                    } catch (IOException e1) {
-                        Logger.logException("Setup socket connection exception", e1);
-                    } finally {
-                        closeServerSocket();
-                        closeConnection();
-                        Logger.logInfo("Close debugserver socket port " + serverPort);
+                    while (true) {
+                        try {
+                            // Allow server socket to service multiple clients at the same time.
+                            // When a request comes in, create a connection thread to process it.
+                            // Then the server goes back to listen for new connection request.
+                            Socket connection = serverSocket.accept();
+                            executor.submit(createConnectionTask(connection));
+                        } catch (IOException e1) {
+                            Logger.logException("Setup socket connection exception", e1);
+                            closeServerSocket();
+                            // If exception occurs when waiting for new client connection, shut down the connection pool
+                            // to make sure no new tasks are accepted. But the previously submitted tasks will continue to run.
+                            shutdownConnectionPool(false);
+                            return;
+                        }
                     }
                 }
 
-            }, "Debug Protocol Server").start();
+            }, "Java Debug Server").start();
         }
     }
 
-    @Override
-    public void stop() {
-        if (protocolServer != null) {
-            protocolServer.stop();
-        }
+    public synchronized void stop() {
+        closeServerSocket();
+        shutdownConnectionPool(true);
     }
 
-    private void closeServerSocket() {
+    private synchronized void closeServerSocket() {
         if (serverSocket != null) {
             try {
+                Logger.logInfo("Close debugserver socket port " + serverSocket.getLocalPort());
                 serverSocket.close();
             } catch (IOException e) {
                 Logger.logException("Close ServerSocket exception", e);
@@ -99,14 +113,32 @@ private void closeServerSocket() {
         serverSocket = null;
     }
 
-    private void closeConnection() {
-        if (connection != null) {
-            try {
-                connection.close();
-            } catch (IOException e) {
-                Logger.logException("Close client socket exception", e);
+    private synchronized void shutdownConnectionPool(boolean now) {
+        if (this.executor != null) {
+            if (now) {
+                this.executor.shutdownNow();
+            } else {
+                this.executor.shutdown();
             }
         }
-        connection = null;
     }
+
+    private Runnable createConnectionTask(Socket connection) {
+        return new Runnable() {
+            public void run() {
+                try {
+                    BufferedReader in = new BufferedReader(new InputStreamReader(connection.getInputStream()));
+                    PrintWriter out = new PrintWriter(connection.getOutputStream(), true);
+                    ProtocolServer protocolServer = new ProtocolServer(in, out, JdtProviderContextFactory.createProviderContext());
+                    // protocol server will dispatch request and send response in a while-loop.
+                    protocolServer.start();
+                } catch (IOException e) {
+                    Logger.logException("Socket connection exception", e);
+                } finally {
+                    Logger.logInfo("Debug connection closed");
+                }
+            }
+        };
+    }
+
 }