Fix shutdown hooks for JUnit5 for JDK24+. Previously, all tests would exit with System.exit or Runtime.exit was called because the shutdown hook was not removed at test teardown.

Author: evanloriot

java/src/com/github/bazel_contrib/contrib_rules_jvm/junit5/JUnit5Runner.java

@@ -87,19 +87,13 @@ private static SystemExitToggle getSystemExitToggle() {
         System.err.println("Failed to load Java 17 system exit override: " + e.getMessage());
       }
 
-      // Install a shutdown hook so people can track down what went wrong
-      // if a test calls `System.exit`
-      Thread shutdownHook = SystemExitDetectingShutdownHook.newShutdownHook(System.err);
-      Runtime.getRuntime().addShutdownHook(shutdownHook);
-
       // Fall through
-    }
+      System.err.println(
+          "Unable to create a mechanism to prevent `System.exit` being called. "
+              + "Tests may cause `bazel test` to exit prematurely.");
 
-    System.err.println(
-        "Unable to create a mechanism to prevent `System.exit` being called. "
-            + "Tests may cause `bazel test` to exit prematurely.");
-
-    return new NullSystemExitToggle();
+      return new NullSystemExitToggle();
+    }
   }
 
   private static void detectJUnit5Classes() {

java/src/com/github/bazel_contrib/contrib_rules_jvm/junit5/NullSystemExitToggle.java

@@ -1,13 +1,32 @@
 package com.github.bazel_contrib.contrib_rules_jvm.junit5;
 
+import java.util.Optional;
+
 public class NullSystemExitToggle implements SystemExitToggle {
+
+  private Optional<Thread> shutdownHook;
+
+  public NullSystemExitToggle() {
+    this.shutdownHook = Optional.empty();
+  }
+
   @Override
   public void prevent() {
-    // No-op
+    if (!shutdownHook.isEmpty()) {
+      throw new RuntimeException("SystemExitDetectingShutdownHook already added");
+    }
+    // Install a shutdown hook so people can track down what went wrong
+    // if a test calls `System.exit`
+    Thread shutdownHook = SystemExitDetectingShutdownHook.newShutdownHook(System.err);
+    this.shutdownHook = Optional.of(shutdownHook);
+    Runtime.getRuntime().addShutdownHook(shutdownHook);
   }
 
   @Override
   public void allow() {
-    // No-op
+    if (shutdownHook.isEmpty()) {
+      throw new RuntimeException("SystemExitDetectingShutdownHook never added");
+    }
+    Runtime.getRuntime().removeShutdownHook(shutdownHook.get());
   }
 }

java/test/com/github/bazel_contrib/contrib_rules_jvm/junit5/NullSystemExitToggleTest.java

@@ -0,0 +1,45 @@
+package com.github.bazel_contrib.contrib_rules_jvm.junit5;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import org.junit.jupiter.api.Test;
+
+public class NullSystemExitToggleTest {
+
+  @Test
+  public void prevent_alreadyPrevented_throwsRuntimeException() {
+    // Arrange.
+    NullSystemExitToggle toggle = new NullSystemExitToggle();
+
+    // Act and Assert.
+    toggle.prevent();
+    assertThrows(RuntimeException.class, () -> toggle.prevent());
+
+    // Cleanup.
+    toggle.allow();
+  }
+
+
+  @Test
+  public void allow_alreadyAllowed_throwsRuntimeException() {
+    // Arrange.
+    NullSystemExitToggle toggle = new NullSystemExitToggle();
+
+    // Act and Assert.
+    assertThrows(RuntimeException.class, () -> toggle.allow());
+  }
+
+  @Test
+  public void normalFlow_success() {
+    // Arrange.
+    NullSystemExitToggle toggle = new NullSystemExitToggle();
+
+    // Act and (implicit) Assert.
+    try {
+      toggle.prevent();
+    } finally {
+      toggle.allow();
+    }
+  }
+
+}