Make SimpleLogHandler not swallow interrupts.

RELNOTES: n/a PiperOrigin-RevId: 346163236
katre · Dec 7, 2020 · f8f66f3 · f8f66f3
1 parent d91e5b4
commit f8f66f3
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 0 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/util/SimpleLogHandler.java b/src/main/java/com/google/devtools/build/lib/util/SimpleLogHandler.java
@@ -26,6 +26,7 @@
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.InterruptedIOException;
 import java.io.OutputStreamWriter;
 import java.lang.management.ManagementFactory;
 import java.nio.file.DirectoryStream;
@@ -413,6 +414,8 @@ public synchronized void publish(LogRecord record) {
       return;
     }
 
+    // This allows us to do the I/O while not forgetting that we were interrupted.
+    boolean isInterrupted = Thread.interrupted();
     try {
       String message = getFormatter().format(record);
       openOutputIfNeeded();
@@ -421,6 +424,9 @@ public synchronized void publish(LogRecord record) {
       reportError(null, e, ErrorManager.WRITE_FAILURE);
       // Failing to log is non-fatal. Continue to try to rotate the log if necessary, which may fix
       // the underlying IO problem with the file.
+      if (e instanceof InterruptedIOException) {
+        isInterrupted = true;
+      }
     }
 
     try {
@@ -430,35 +436,58 @@ public synchronized void publish(LogRecord record) {
       }
     } catch (IOException e) {
       reportError("Failed to rotate log file", e, ErrorManager.GENERIC_FAILURE);
+      if (e instanceof InterruptedIOException) {
+        isInterrupted = true;
+      }
+    }
+    if (isInterrupted) {
+      Thread.currentThread().interrupt();
     }
   }
 
   @Override
   public synchronized void flush() {
+    boolean isInterrupted = Thread.interrupted();
     if (output.isOpen()) {
       try {
         output.flush();
       } catch (IOException e) {
         reportError(null, e, ErrorManager.FLUSH_FAILURE);
+        if (e instanceof InterruptedIOException) {
+          isInterrupted = true;
+        }
       }
     }
+    if (isInterrupted) {
+      Thread.currentThread().interrupt();
+    }
   }
 
   @Override
   public synchronized void close() {
+    boolean isInterrupted = Thread.interrupted();
     if (output.isOpen()) {
       try {
         output.write(getFormatter().getTail(this));
       } catch (IOException e) {
         reportError("Failed to write log tail", e, ErrorManager.WRITE_FAILURE);
+        if (e instanceof InterruptedIOException) {
+          isInterrupted = true;
+        }
       }
 
       try {
         output.close();
       } catch (IOException e) {
         reportError(null, e, ErrorManager.CLOSE_FAILURE);
+        if (e instanceof InterruptedIOException) {
+          isInterrupted = true;
+        }
       }
     }
+    if (isInterrupted) {
+      Thread.currentThread().interrupt();
+    }
   }
 
   /**

diff --git a/src/test/java/com/google/devtools/build/lib/util/SimpleLogHandlerTest.java b/src/test/java/com/google/devtools/build/lib/util/SimpleLogHandlerTest.java
@@ -498,4 +498,26 @@ public void getLoggerFilePath_onMissingLogHandler_fails() throws Exception {
 
     assertThrows(IOException.class, () -> handlerQuerier.getLoggerFilePath(logger));
   }
+
+  @Test
+  public void publish_handlesInterrupt() throws Exception {
+    SimpleLogHandler handler =
+        SimpleLogHandler.builder()
+            .setPrefix(tmp.getRoot() + File.separator + "hello")
+            .setFormatter(new TrivialFormatter())
+            .build();
+    Thread t =
+        new Thread(
+            () -> {
+              Thread.currentThread().interrupt();
+              handler.publish(new LogRecord(Level.SEVERE, "Hello world")); // To open the log file.
+              assertThat(Thread.currentThread().isInterrupted()).isTrue();
+              handler.flush();
+              assertThat(Thread.currentThread().isInterrupted()).isTrue();
+              handler.close();
+              assertThat(Thread.currentThread().isInterrupted()).isTrue();
+            });
+    t.run();
+    t.join();
+  }
 }