Add a shutdown hook for tests that detects System.exit.

We can no longer prevent code being tested from exiting when `System.exit` is called, but we can at least show where it happened. We do this by the examining the stack traces of all threads in a shutdown hook, looking for `Runtime.exit`. (`System.exit` calls `Runtime.exit`, and users can also call it directly.)

We cannot detect `Runtime.halt` this way, unfortunately.

Recent JDK versions can be configured to log where `System.exit` is called, but that logging is fiddly and its configuration may interfere with that of the code under test. Also, we may later add to the shutdown hook so that it also detects which test method is currently executing, in case that isn't in the thread that called `System.exit`.

PiperOrigin-RevId: 703190140
Change-Id: Ie4ba4728c41adb868f7fc75b630851b900e7e27d

src/java_tools/junitrunner/java/com/google/testing/junit/runner/internal/BUILD

@@ -12,13 +12,21 @@ package(
 # lives in the ":junit4" rule.
 java_library(
     name = "internal",
-    srcs = glob(["*.java"]),
+    srcs = glob(
+        ["*.java"],
+        exclude = ["SystemExitDetectingShutdownHook.java"],
+    ),
     deps = [
         "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/util",
         "//third_party:jsr330_inject",
     ],
 )
 
+java_library(
+    name = "system_exit_detecting_shutdown_hook",
+    srcs = ["SystemExitDetectingShutdownHook.java"],
+)
+
 # Internal code for the JUnit runner that depends on JUnit 4.
 # Code used by the JUnit runner that doesn't depend on JUnit 4
 # lives in the "internal" rule.

src/java_tools/junitrunner/java/com/google/testing/junit/runner/internal/SystemExitDetectingShutdownHook.java

@@ -0,0 +1,68 @@
+// Copyright 2024 The Bazel Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.package com.google.testing.junit.runner.internal;
+
+package com.google.testing.junit.runner.internal;
+
+import static java.util.Arrays.stream;
+import static java.util.stream.Collectors.toList;
+
+import java.io.PrintStream;
+import java.util.List;
+
+/**
+ * Shutdown hook to detect when the shutdown is due to someone calling {@code System.exit}. Tests
+ * should never do that. Previously we had a security manager that intercepted such calls. The JDK
+ * will remove security managers in a future release, so instead we just detect when it happens and
+ * print a stack trace so users can find and fix the call.
+ */
+public class SystemExitDetectingShutdownHook {
+  public static Thread newShutdownHook(PrintStream testRunnerOut) {
+    Runnable hook = () -> {
+      boolean foundRuntimeExit = false;
+      for (StackTraceElement[] stack : Thread.getAllStackTraces().values()) {
+        @SuppressWarnings("JdkCollectors") // can't use ImmutableList here
+        List<String> framesStartingWithRuntimeExit =
+            stream(stack)
+                .dropWhile(
+                    frame ->
+                        !frame.getClassName().equals("java.lang.Runtime")
+                            || !frame.getMethodName().equals("exit"))
+                .map(SystemExitDetectingShutdownHook::frameString)
+                .collect(toList());
+        if (!framesStartingWithRuntimeExit.isEmpty()) {
+          foundRuntimeExit = true;
+          testRunnerOut.println("\nSystem.exit or Runtime.exit was called!");
+          testRunnerOut.println(String.join("\n", framesStartingWithRuntimeExit));
+        }
+      }
+      if (foundRuntimeExit) {
+        // We must call halt rather than exit, because exit would lead to a deadlock. We use a
+        // hopefully unique exit code to make it easier to identify this case.
+        Runtime.getRuntime().halt(121);
+      }
+    };
+    return new Thread(hook, "SystemExitDetectingShutdownHook");
+  }
+
+  private static String frameString(StackTraceElement frame) {
+    return String.format(
+        "        at %s.%s(%s:%d)",
+        frame.getClassName(),
+        frame.getMethodName(),
+        frame.getFileName(),
+        frame.getLineNumber());
+  }
+
+  private SystemExitDetectingShutdownHook() {}
+}

src/java_tools/junitrunner/java/com/google/testing/junit/runner/junit4/BUILD

@@ -15,6 +15,7 @@ java_library(
         "//src/java_tools/junitrunner/java/com/google/testing/junit/junit4:runner",
         "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/internal",
         "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/internal:junit4",
+        "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/internal:system_exit_detecting_shutdown_hook",
         "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/model",
         "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/sharding",
         "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/util",

src/java_tools/junitrunner/java/com/google/testing/junit/runner/junit4/JUnit4Runner.java

@@ -17,6 +17,7 @@
 import com.google.testing.junit.junit4.runner.RegExTestCaseFilter;
 import com.google.testing.junit.junit4.runner.SuiteTrimmingFilter;
 import com.google.testing.junit.runner.internal.Stdout;
+import com.google.testing.junit.runner.internal.SystemExitDetectingShutdownHook;
 import com.google.testing.junit.runner.internal.junit4.CancellableRequestFactory;
 import com.google.testing.junit.runner.model.TestSuiteModel;
 import java.io.File;
@@ -103,11 +104,14 @@ public Result run() {
 
     File exitFile = getExitFile();
     exitFileActive(exitFile);
+    Thread shutdownHook = SystemExitDetectingShutdownHook.newShutdownHook(testRunnerOut);
+    Runtime.getRuntime().addShutdownHook(shutdownHook);
     try {
       Request cancellableRequest = requestFactory.createRequest(filteredRequest);
       return core.run(cancellableRequest);
     } finally {
       exitFileInactive(exitFile);
+      Runtime.getRuntime().removeShutdownHook(shutdownHook);
     }
   }
 

src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/BUILD

@@ -21,6 +21,7 @@ java_library(
     deps = [
         "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/internal",
         "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/internal:junit4",
+        "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/internal:system_exit_detecting_shutdown_hook",
         "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/junit4",
         "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/model",
         "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/sharding",
@@ -134,9 +135,37 @@ sh_test(
     toolchains = ["@bazel_tools//tools/jdk:current_java_runtime"],
 )
 
+java_binary(
+    name = "ProgramThatCallsSystemExit",
+    srcs = ["ProgramThatCallsSystemExit.java"],
+    deps = [
+        "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/internal:system_exit_detecting_shutdown_hook",
+    ],
+)
+
+sh_test(
+    name = "system_exit_detecting_test",
+    srcs = ["system_exit_detecting_test.sh"],
+    args = [
+        "$(location //src/test/shell:unittest.bash)",
+        "$(location :ProgramThatCallsSystemExit_deploy.jar)",
+        "$(JAVABASE)",
+        "$(location :testdata/system_exit_detecting_test_stack.txt)",
+    ],
+    data = [
+        ":ProgramThatCallsSystemExit_deploy.jar",
+        ":testdata/system_exit_detecting_test_stack.txt",
+        "//src/test/shell:bashunit",
+        "//src/test/shell:unittest.bash",
+        "@bazel_tools//tools/jdk:current_java_runtime",
+    ],
+    toolchains = ["@bazel_tools//tools/jdk:current_java_runtime"],
+)
+
 exports_files([
     "stack_trace_integration_tests.sh",
     "junit4_testbridge_integration_tests.sh",
     "utf8_test_log_test.sh",
     "deploy_jar_integration_tests.sh",
+    "system_exit_detecting_test.sh",
 ])

src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/ProgramThatCallsSystemExit.java

@@ -0,0 +1,32 @@
+// Copyright 2024 The Bazel Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.testing.junit.runner;
+
+import com.google.testing.junit.runner.internal.SystemExitDetectingShutdownHook;
+
+/**
+ * A simple program that installs a shutdown hook using {@link SystemExitDetectingShutdownHook} and
+ * then calls {@code System.exit}. This is used to test that the shutdown hook detects the {@code
+ * System.exit} call and prints a stack trace.
+ */
+public final class ProgramThatCallsSystemExit {
+  public static void main(String[] args) {
+    Thread shutdownHook = SystemExitDetectingShutdownHook.newShutdownHook(System.err);
+    Runtime.getRuntime().addShutdownHook(shutdownHook);
+    System.exit(0);
+  }
+
+  private ProgramThatCallsSystemExit() {}
+}

src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/system_exit_detecting_test.sh

@@ -0,0 +1,50 @@
+#!/bin/bash
+#
+# Copyright 2024 The Bazel Authors. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# Tests that the SystemExitDetectingShutdownHook correctly detects a call to
+# System.exit and prints a stack trace.
+
+[ -z "$TEST_SRCDIR" ] && { echo "TEST_SRCDIR not set!" >&2; exit 1; }
+
+# Load the unit-testing framework
+source "$1" || \
+  { echo "Failed to load unit-testing framework $1" >&2; exit 1; }
+
+set +o errexit
+
+PROGRAM_THAT_CALLS_SYSTEM_EXIT_JAR="$2"
+readonly PROGRAM_THAT_CALLS_SYSTEM_EXIT_JAR
+JAVA_HOME="$3"
+readonly JAVA_HOME
+EXPECTED_STACK_FILE="$4"
+readonly EXPECTED_STACK_FILE
+
+function test_prints_stack_trace_on_system_exit() {
+  local output_file="${TEST_TMPDIR}/output.txt"
+
+  "${JAVA_HOME}/bin/java" -jar "${PROGRAM_THAT_CALLS_SYSTEM_EXIT_JAR}" \
+      2> "${output_file}"
+  assert_equals 121 $?
+
+  # We expect the output to be a stack trace that ends with the main method of
+  # ProgramThatCallsSystemExit. We use sed to avoid hardcoding the exact line
+  # numbers in the stack trace.
+  sed -i 's/:[0-9][0-9]*/:XXX/' "${output_file}"
+  diff -u "${output_file}" "${EXPECTED_STACK_FILE}" || \
+    fail "Stack trace does not match expected stack trace"
+}
+
+run_suite "system_exit_detecting_test"

src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testdata/system_exit_detecting_test_stack.txt

@@ -0,0 +1,5 @@
+
+System.exit or Runtime.exit was called!
+        at java.lang.Runtime.exit(Runtime.java:XXX)
+        at java.lang.System.exit(System.java:XXX)
+        at com.google.testing.junit.runner.ProgramThatCallsSystemExit.main(ProgramThatCallsSystemExit.java:XXX)