From cef9fff0675c85906e134e4c0a8d2036860cd4b3 Mon Sep 17 00:00:00 2001
From: Wojciech Kudla <wojciech.kudla@hsbc.com>
Date: Tue, 5 Sep 2023 14:12:45 +0000
Subject: [PATCH] 8305507: Add support for grace period before
 AbortVMOnSafepointTimeout triggers

Reviewed-by: dholmes, pchilanomate
---
 src/hotspot/share/runtime/globals.hpp         |  4 ++
 src/hotspot/share/runtime/safepoint.cpp       |  2 +-
 .../TestAbortVMOnSafepointTimeout.java        | 55 ++++++++++++++++++-
 3 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/src/hotspot/share/runtime/globals.hpp b/src/hotspot/share/runtime/globals.hpp
index 725ded4bd7a..4c2d19469ff 100644
--- a/src/hotspot/share/runtime/globals.hpp
+++ b/src/hotspot/share/runtime/globals.hpp
@@ -430,6 +430,10 @@ const int ObjectAlignmentInBytes = 8;
   product(bool, AbortVMOnSafepointTimeout, false, DIAGNOSTIC,               \
           "Abort upon failure to reach safepoint (see SafepointTimeout)")   \
                                                                             \
+  product(uint64_t, AbortVMOnSafepointTimeoutDelay, 0, DIAGNOSTIC,          \
+          "Delay in milliseconds for option AbortVMOnSafepointTimeout")     \
+          range(0, max_jlong)                                               \
+                                                                            \
   product(bool, AbortVMOnVMOperationTimeout, false, DIAGNOSTIC,             \
           "Abort upon failure to complete VM operation promptly")           \
                                                                             \
diff --git a/src/hotspot/share/runtime/safepoint.cpp b/src/hotspot/share/runtime/safepoint.cpp
index c826119910b..266e6b54aca 100644
--- a/src/hotspot/share/runtime/safepoint.cpp
+++ b/src/hotspot/share/runtime/safepoint.cpp
@@ -808,7 +808,7 @@ void SafepointSynchronize::print_safepoint_timeout() {
 
   // To debug the long safepoint, specify both AbortVMOnSafepointTimeout &
   // ShowMessageBoxOnError.
-  if (AbortVMOnSafepointTimeout) {
+  if (AbortVMOnSafepointTimeout && (os::elapsedTime() * MILLIUNITS > AbortVMOnSafepointTimeoutDelay)) {
     // Send the blocking thread a signal to terminate and write an error file.
     for (JavaThreadIteratorWithHandle jtiwh; JavaThread *cur_thread = jtiwh.next(); ) {
       if (cur_thread->safepoint_state()->is_running()) {
diff --git a/test/hotspot/jtreg/runtime/Safepoint/TestAbortVMOnSafepointTimeout.java b/test/hotspot/jtreg/runtime/Safepoint/TestAbortVMOnSafepointTimeout.java
index e7e3cfca781..aadb61fd5b0 100644
--- a/test/hotspot/jtreg/runtime/Safepoint/TestAbortVMOnSafepointTimeout.java
+++ b/test/hotspot/jtreg/runtime/Safepoint/TestAbortVMOnSafepointTimeout.java
@@ -28,7 +28,8 @@ import jdk.test.whitebox.WhiteBox;
 
 /*
  * @test TestAbortVMOnSafepointTimeout
- * @summary Check if VM can kill thread which doesn't reach safepoint.
+ * @summary Check if VM can kill thread which doesn't reach safepoint,
+ *          test grace period before AbortVMOnSafepointTimeout kicks in
  * @bug 8219584 8227528
  * @requires vm.flagless
  * @library /testlibrary /test/lib
@@ -39,7 +40,7 @@ import jdk.test.whitebox.WhiteBox;
 
 public class TestAbortVMOnSafepointTimeout {
 
-    public static void main(String[] args) throws Exception {
+    public static void testThreadKilledOnSafepointTimeout() throws Exception {
         ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
                 "-Xbootclasspath/a:.",
                 "-XX:+UnlockDiagnosticVMOptions",
@@ -56,6 +57,31 @@ public class TestAbortVMOnSafepointTimeout {
         );
 
         OutputAnalyzer output = new OutputAnalyzer(pb.start());
+        verifyAbortVmApplied(output);
+    }
+
+    public static void testGracePeriodAppliedBeforeVmAbort() throws Exception {
+        ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
+                "-Xbootclasspath/a:.",
+                "-XX:+UnlockDiagnosticVMOptions",
+                "-XX:+WhiteBoxAPI",
+                "-XX:+SafepointTimeout",
+                "-XX:+SafepointALot",
+                "-XX:+AbortVMOnSafepointTimeout",
+                "-XX:AbortVMOnSafepointTimeoutDelay=2500",
+                "-XX:SafepointTimeoutDelay=50",
+                "-XX:GuaranteedSafepointInterval=1",
+                "-XX:-CreateCoredumpOnCrash",
+                "-Xms64m",
+                "TestAbortVMOnSafepointTimeout$TestWithDelay"
+        );
+
+        OutputAnalyzer output = new OutputAnalyzer(pb.start());
+        output.shouldContain(TestWithDelay.PRE_STALL_TEXT);
+        verifyAbortVmApplied(output);
+    }
+
+    private static void verifyAbortVmApplied(OutputAnalyzer output) {
         output.shouldContain("Timed out while spinning to reach a safepoint.");
         if (Platform.isWindows()) {
             output.shouldContain("Safepoint sync time longer than");
@@ -68,6 +94,14 @@ public class TestAbortVMOnSafepointTimeout {
         output.shouldNotHaveExitValue(0);
     }
 
+    public static void main(String[] args) throws Exception {
+        // test basic AbortVMOnSafepointTimeout functionality
+        testThreadKilledOnSafepointTimeout();
+
+        // verify -XX:AbortVMOnSafepointTimeoutDelay functionality
+        testGracePeriodAppliedBeforeVmAbort();
+    }
+
     public static class Test {
         public static void main(String[] args) throws Exception {
             Integer waitTime = Integer.parseInt(args[0]);
@@ -78,4 +112,21 @@ public class TestAbortVMOnSafepointTimeout {
             }
         }
     }
+
+    public static class TestWithDelay {
+
+        public static final String PRE_STALL_TEXT = "THE FOLLOWING STALL SHOULD BE CAPTURED";
+
+        public static void main(String[] args) throws Exception {
+            WhiteBox wb = WhiteBox.getWhiteBox();
+            // induce a stall that should not be picked up before grace period
+            wb.waitUnsafe(999);
+            System.out.println(PRE_STALL_TEXT);
+
+            // trigger safepoint timeout
+            while (true) {
+                wb.waitUnsafe(999);
+            }
+        }
+    }
 }