Add the skylark debugging options to the recognized 'build' options.

Ensure debugging is turned off (and the server socket closed) when
the command finishes, even if the blaze server is shut down.

Finally, unpause all threads as part of shutting down the debug server,
and also shut down the server if the client connection is lost.

PiperOrigin-RevId: 199863623

4 files changed, 42 insertions, 9 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/module/BUILD b/src/main/java/com/google/devtools/build/lib/skylarkdebug/module/BUILD
index 0fc7f18c04..39c6b04031 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkdebug/module/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/module/BUILD
@@ -15,6 +15,8 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib:runtime",
         "//src/main/java/com/google/devtools/build/lib:syntax",
         "//src/main/java/com/google/devtools/build/lib/skylarkdebug/server",
+        "//src/main/java/com/google/devtools/common/options",
+        "//third_party:guava",
     ],
 )
 
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/module/SkylarkDebuggerModule.java b/src/main/java/com/google/devtools/build/lib/skylarkdebug/module/SkylarkDebuggerModule.java
index 8dbe9e020a..d4f74e4d6e 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkdebug/module/SkylarkDebuggerModule.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/module/SkylarkDebuggerModule.java
@@ -14,12 +14,15 @@
 
 package com.google.devtools.build.lib.skylarkdebug.module;
 
+import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.runtime.BlazeModule;
+import com.google.devtools.build.lib.runtime.Command;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.skylarkdebug.server.SkylarkDebugServer;
 import com.google.devtools.build.lib.syntax.DebugServerUtils;
+import com.google.devtools.common.options.OptionsBase;
 import java.io.IOException;
 
 /** Blaze module for setting up Skylark debugging. */
@@ -41,6 +44,23 @@ public final class SkylarkDebuggerModule extends BlazeModule {
     disableDebugging();
   }
 
+  @Override
+  public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
+    return "build".equals(command.name())
+        ? ImmutableList.of(SkylarkDebuggerOptions.class)
+        : ImmutableList.of();
+  }
+
+  @Override
+  public void blazeShutdown() {
+    disableDebugging();
+  }
+
+  @Override
+  public void blazeShutdownOnCrash() {
+    disableDebugging();
+  }
+
   private static void initializeDebugging(Reporter reporter, int debugPort) {
     try {
       SkylarkDebugServer server =
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/DebugServerTransport.java b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/DebugServerTransport.java
index 0763fd5047..a1696e368a 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/DebugServerTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/DebugServerTransport.java
@@ -71,19 +71,25 @@ final class DebugServerTransport {
   @Nullable
   DebugRequest readClientRequest() {
     synchronized (requestStream) {
-      while (true) {
-        if (!clientSocket.isConnected() || clientSocket.isClosed()) {
-          return null;
-        }
-        try {
-          return DebugRequest.parseDelimitedFrom(requestStream);
-        } catch (IOException e) {
-          postEvent(DebugEventHelper.error("Error parsing debug request: " + e.getMessage()));
-        }
+      try {
+        return DebugRequest.parseDelimitedFrom(requestStream);
+      } catch (IOException e) {
+        handleParsingError(e);
+        return null;
       }
     }
   }
 
+  private void handleParsingError(IOException e) {
+    if (isClosed()) {
+      // an IOException is expected when the client disconnects -- no need to log an error
+      return;
+    }
+    String message = "Error parsing debug request: " + e.getMessage();
+    postEvent(DebugEventHelper.error(message));
+    eventHandler.handle(Event.error(message));
+  }
+
   /** Posts a debug event. */
   void postEvent(DebugEvent event) {
     synchronized (eventStream) {
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServer.java b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServer.java
index 71a3d7b41f..00af706a47 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServer.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServer.java
@@ -103,6 +103,8 @@ public final class SkylarkDebugServer implements DebugServer {
                           "Debug server listener thread died: "
                               + Throwables.getStackTraceAsString(e)));
                 }
+              } finally {
+                close();
               }
             });
 
@@ -119,6 +121,9 @@ public final class SkylarkDebugServer implements DebugServer {
       eventHandler.handle(
           Event.error(
               "Error shutting down the debug server: " + Throwables.getStackTraceAsString(e)));
+    } finally {
+      // ensure no threads are left paused, otherwise the build command will never complete
+      threadHandler.resumeAllThreads();
     }
   }