Fix debug buttons disabled issue: send StoppedEvent after StepResponse/ContinueResponse (#227)

* Fix debug buttons disabled issue: send StoppeEvent after StepResponse/ContinuedResponse

Signed-off-by: Jinbo Wang <[email protected]>

* Remove the incorrect comments

* Use private object as lock

Author: testforstephen

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/ProtocolServer.java

@@ -15,13 +15,16 @@
 import java.io.OutputStream;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
+import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import com.microsoft.java.debug.core.Configuration;
 import com.microsoft.java.debug.core.DebugException;
 import com.microsoft.java.debug.core.UsageDataSession;
 import com.microsoft.java.debug.core.protocol.AbstractProtocolServer;
+import com.microsoft.java.debug.core.protocol.Events.DebugEvent;
+import com.microsoft.java.debug.core.protocol.Events.StoppedEvent;
 import com.microsoft.java.debug.core.protocol.Messages;
 import com.sun.jdi.VMDisconnectedException;
 
@@ -31,6 +34,10 @@ public class ProtocolServer extends AbstractProtocolServer {
     private IDebugAdapter debugAdapter;
     private UsageDataSession usageDataSession = new UsageDataSession();
 
+    private Object lock = new Object();
+    private boolean isDispatchingRequest = false;
+    private ConcurrentLinkedQueue<DebugEvent> eventQueue = new ConcurrentLinkedQueue<>();
+
     /**
      * Constructs a protocol server instance based on the given input stream and output stream.
      * @param input
@@ -75,43 +82,83 @@ public CompletableFuture<Messages.Response> sendRequest(Messages.Request request
     }
 
     @Override
-    protected void dispatchRequest(Messages.Request request) {
-        usageDataSession.recordRequest(request);
-        debugAdapter.dispatchRequest(request).thenCompose((response) -> {
-            CompletableFuture<Void> future = new CompletableFuture<>();
-            if (response != null) {
-                sendResponse(response);
-                future.complete(null);
+    public void sendEvent(DebugEvent event) {
+        // See the two bugs https://github.com/Microsoft/java-debug/issues/134 and https://github.com/Microsoft/vscode/issues/58327,
+        // it requires the java-debug to send the StoppedEvent after ContinueResponse/StepResponse is received by DA.
+        if (event instanceof StoppedEvent) {
+            sendEventLater(event);
+        } else {
+            super.sendEvent(event);
+        }
+
+    }
+
+    /**
+     * If the the dispatcher is idle, then send the event to the DA immediately.
+     * Else add the new event to an eventQueue first and send them when dispatcher becomes idle again.
+     */
+    private void sendEventLater(DebugEvent event) {
+        synchronized (lock) {
+            if (this.isDispatchingRequest) {
+                this.eventQueue.offer(event);
             } else {
-                future.completeExceptionally(new DebugException("The request dispatcher should not return null response.",
-                    ErrorCode.UNKNOWN_FAILURE.getId()));
+                super.sendEvent(event);
             }
-            return future;
-        }).exceptionally((ex) -> {
-            Messages.Response response = new Messages.Response(request.seq, request.command);
-            if (ex instanceof CompletionException && ex.getCause() != null) {
-                ex = ex.getCause();
+        }
+    }
+
+    @Override
+    protected void dispatchRequest(Messages.Request request) {
+        usageDataSession.recordRequest(request);
+        try {
+            synchronized (lock) {
+                this.isDispatchingRequest = true;
             }
 
-            if (ex instanceof VMDisconnectedException) {
-                // mark it success to avoid reporting error on VSCode.
-                response.success = true;
-                sendResponse(response);
-            } else {
-                String exceptionMessage = ex.getMessage() != null ? ex.getMessage() : ex.toString();
-                ErrorCode errorCode = ex instanceof DebugException ? ErrorCode.parse(((DebugException) ex).getErrorCode()) : ErrorCode.UNKNOWN_FAILURE;
-                boolean isUserError = ex instanceof DebugException && ((DebugException) ex).isUserError();
-                if (isUserError) {
-                    usageDataSession.recordUserError(errorCode);
+            debugAdapter.dispatchRequest(request).thenCompose((response) -> {
+                CompletableFuture<Void> future = new CompletableFuture<>();
+                if (response != null) {
+                    sendResponse(response);
+                    future.complete(null);
+                } else {
+                    future.completeExceptionally(new DebugException("The request dispatcher should not return null response.",
+                            ErrorCode.UNKNOWN_FAILURE.getId()));
+                }
+                return future;
+            }).exceptionally((ex) -> {
+                Messages.Response response = new Messages.Response(request.seq, request.command);
+                if (ex instanceof CompletionException && ex.getCause() != null) {
+                    ex = ex.getCause();
+                }
+
+                if (ex instanceof VMDisconnectedException) {
+                    // mark it success to avoid reporting error on VSCode.
+                    response.success = true;
+                    sendResponse(response);
                 } else {
-                    logger.log(Level.SEVERE, String.format("[error response][%s]: %s", request.command, exceptionMessage), ex);
+                    String exceptionMessage = ex.getMessage() != null ? ex.getMessage() : ex.toString();
+                    ErrorCode errorCode = ex instanceof DebugException ? ErrorCode.parse(((DebugException) ex).getErrorCode()) : ErrorCode.UNKNOWN_FAILURE;
+                    boolean isUserError = ex instanceof DebugException && ((DebugException) ex).isUserError();
+                    if (isUserError) {
+                        usageDataSession.recordUserError(errorCode);
+                    } else {
+                        logger.log(Level.SEVERE, String.format("[error response][%s]: %s", request.command, exceptionMessage), ex);
+                    }
+
+                    sendResponse(AdapterUtils.setErrorResponse(response,
+                            errorCode,
+                            exceptionMessage));
                 }
+                return null;
+            }).join();
+        } finally {
+            synchronized (lock) {
+                this.isDispatchingRequest = false;
+            }
 
-                sendResponse(AdapterUtils.setErrorResponse(response,
-                    errorCode,
-                    exceptionMessage));
+            while (this.eventQueue.peek() != null) {
+                super.sendEvent(this.eventQueue.poll());
             }
-            return null;
-        }).join();
+        }
     }
 }

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/RestartFrameHandler.java

@@ -106,8 +106,8 @@ private void stepInto(IDebugAdapterContext context, ThreadReference thread) {
         context.getDebugSession().getEventHub().stepEvents().filter(debugEvent -> request.equals(debugEvent.event.request())).take(1).subscribe(debugEvent -> {
             debugEvent.shouldResume = false;
             // Have to send two events to keep the UI sync with the step in operations:
-            context.getProtocolServer().sendEvent(new Events.StoppedEvent("restartframe", thread.uniqueID()));
             context.getProtocolServer().sendEvent(new Events.ContinuedEvent(thread.uniqueID()));
+            context.getProtocolServer().sendEvent(new Events.StoppedEvent("restartframe", thread.uniqueID()));
         });
         request.enable();
         thread.resume();