8385651: HotCodeSampler crashes with JFR enabled

Co-authored-by: Evgeny Astigeevich <eastigeevich@openjdk.org>
Reviewed-by: mgronlun, kvn

src/hotspot/os/posix/signals_posix.cpp

@@ -1881,6 +1881,12 @@ void PosixSignals::do_resume(OSThread* osthread) {
 }
 
 void SuspendedThreadTask::internal_do_task() {
+#if INCLUDE_JFR
+  assert(NOT_COMPILER2(true) COMPILER2_PRESENT(!HotCodeHeap) ||
+         SuspendedThreadTask_lock->owned_by_self(),
+         "suspend/resume must be serialized when HotCodeHeap is enabled");
+#endif
+
   if (PosixSignals::do_suspend(_thread->osthread())) {
     SuspendedThreadTaskContext context(_thread, _thread->osthread()->ucontext());
     do_task(context);

src/hotspot/os/windows/os_windows.cpp

@@ -6058,6 +6058,11 @@ static inline HANDLE get_thread_handle_for_extended_context(DWORD tid) {
 // Thread sampling implementation
 //
 void SuspendedThreadTask::internal_do_task() {
+#if INCLUDE_JFR
+  assert(NOT_COMPILER2(true) COMPILER2_PRESENT(!HotCodeHeap) ||
+         SuspendedThreadTask_lock->owned_by_self(),
+         "suspend/resume must be serialized when HotCodeHeap is enabled");
+#endif
   const HANDLE h = get_thread_handle_for_extended_context(_thread->osthread()->thread_id());
   if (h == nullptr) {
     return;

src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp

@@ -32,6 +32,9 @@
 #include "jfr/utilities/jfrTryLock.hpp"
 #include "jfr/utilities/jfrTypes.hpp"
 #include "logging/log.hpp"
+#ifdef COMPILER2
+#include "opto/c2_globals.hpp"
+#endif
 #include "runtime/atomicAccess.hpp"
 #include "runtime/globals.hpp"
 #include "runtime/javaThread.inline.hpp"
@@ -288,7 +291,19 @@ class OSThreadSampler : public SuspendedThreadTask {
  public:
   OSThreadSampler(JavaThread* jt) : SuspendedThreadTask(jt),
                                     _result(THREAD_SUSPENSION_ERROR) {}
-  void request_sample() { run(); }
+  void request_sample() {
+#ifdef COMPILER2
+    if (HotCodeHeap) {
+      JfrMutexTryLock try_lock(SuspendedThreadTask_lock);
+      if (try_lock.acquired()) {
+        run();
+      }
+      return;
+    }
+#endif
+    run();
+  }
+
   JfrSampleResult result() const { return _result; }
 
   void do_task(const SuspendedThreadTaskContext& context) {

src/hotspot/share/runtime/hotCodeCollector.cpp

@@ -97,7 +97,9 @@ void HotCodeCollector::thread_entry(JavaThread* thread, TRAPS) {
       ThreadSampler sampler;
       uint64_t start_time = os::javaTimeMillis();
       while (os::javaTimeMillis() - start_time <= HotCodeSampleSeconds * 1000) {
-        sampler.sample_all_java_threads();
+        if (!sampler.sample_all_java_threads()) {
+          break;
+        }
         thread->sleep(rand_sampling_period_ms());
       }
 

src/hotspot/share/runtime/hotCodeSampler.cpp

@@ -29,7 +29,13 @@
 #include "runtime/hotCodeSampler.hpp"
 #include "runtime/javaThread.inline.hpp"
 
-void ThreadSampler::sample_all_java_threads() {
+#if INCLUDE_JFR
+#include "jfr/utilities/jfrTryLock.hpp"
+
+using SuspendedThreadTaskTryLock = JfrMutexTryLock;
+#endif
+
+bool ThreadSampler::sample_all_java_threads() {
   // Collect samples for each JavaThread
   for (JavaThreadIteratorWithHandle jtiwh; JavaThread *jt = jtiwh.next(); ) {
     if (jt->is_hidden_from_external_view() ||
@@ -39,7 +45,17 @@ void ThreadSampler::sample_all_java_threads() {
     }
 
     GetPCTask task(jt);
-    task.run();
+    {
+#if INCLUDE_JFR
+      SuspendedThreadTaskTryLock try_lock(SuspendedThreadTask_lock);
+      if (!try_lock.acquired()) {
+        log_debug(hotcode)("Suspend lock held by JFR sampler; stopping this sampling round, will retry after %u seconds", HotCodeIntervalSeconds);
+        return false;
+      }
+#endif
+      task.run();
+    }
+
     address pc = task.pc();
     if (pc == nullptr) {
       continue;
@@ -57,6 +73,7 @@ void ThreadSampler::sample_all_java_threads() {
       }
     }
   }
+  return true;
 }
 
 Candidates::Candidates(ThreadSampler& sampler)

src/hotspot/share/runtime/hotCodeSampler.hpp

@@ -90,8 +90,8 @@ class ThreadSampler : public StackObj {
  public:
   ThreadSampler() : _samples(INITIAL_TABLE_SIZE, HotCodeSampleSeconds * 1000 / HotCodeMaxSamplingMs) {}
 
-  // Iterate over and sample all Java threads
-  void sample_all_java_threads();
+  // Iterate over and sample all Java threads. Return false if sampling was interrupted by JFR sampling.
+  bool sample_all_java_threads();
 
   // Iterate over all samples with a callback function
   template<typename Function>

src/hotspot/share/runtime/mutexLocker.cpp

@@ -121,6 +121,7 @@ Mutex*   Verify_lock                  = nullptr;
 Mutex*   JfrStacktrace_lock           = nullptr;
 Monitor* JfrMsg_lock                  = nullptr;
 Mutex*   JfrBuffer_lock               = nullptr;
+Mutex*   SuspendedThreadTask_lock     = nullptr;
 #endif
 
 Mutex*   CodeHeapStateAnalytics_lock  = nullptr;
@@ -280,6 +281,7 @@ void mutex_init() {
   MUTEX_DEFN(JfrBuffer_lock                  , PaddedMutex  , event);
   MUTEX_DEFN(JfrMsg_lock                     , PaddedMonitor, event);
   MUTEX_DEFN(JfrStacktrace_lock              , PaddedMutex  , event);
+  MUTEX_DEFN(SuspendedThreadTask_lock        , PaddedMutex  , nosafepoint);
 #endif
 
   MUTEX_DEFN(ContinuationRelativize_lock     , PaddedMonitor, nosafepoint-3);

src/hotspot/share/runtime/mutexLocker.hpp

@@ -139,6 +139,7 @@ extern Mutex*   FinalImageRecipes_lock;          // Protecting the tables used b
 extern Mutex*   JfrStacktrace_lock;              // used to guard access to the JFR stacktrace table
 extern Monitor* JfrMsg_lock;                     // protects JFR messaging
 extern Mutex*   JfrBuffer_lock;                  // protects JFR buffer operations
+extern Mutex*   SuspendedThreadTask_lock;        // used to guard SuspendedThreadTask::run
 #endif
 
 extern Mutex*   Metaspace_lock;                  // protects Metaspace virtualspace and chunk expansions

test/hotspot/jtreg/compiler/hotcode/HotCodeCollectorJFR.java

@@ -0,0 +1,46 @@
+/*
+ * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+/*
+ * @test
+ * @bug 8385651
+ * @summary Verify the HotCodeSampler and JFR do not attempt to suspend the same JavaThread and crash
+ * @requires vm.compiler2.enabled & vm.hasJFR
+ * @run main/othervm -XX:StartFlightRecording -XX:+UnlockExperimentalVMOptions -XX:+HotCodeHeap -XX:+NMethodRelocation -XX:+UnlockDiagnosticVMOptions
+ *                   -XX:HotCodeIntervalSeconds=0 -XX:HotCodeStartupDelaySeconds=0 -XX:HotCodeStablePercent=-1 -Xlog:hotcode=debug
+ *                   compiler.hotcode.HotCodeCollectorJFR
+ */
+
+package compiler.hotcode;
+
+public class HotCodeCollectorJFR {
+
+    private static final int FUNC_RUN_MILLIS = 10_000;
+
+    public static void main(String[] args) throws Exception {
+        long start = System.currentTimeMillis();
+        while (System.currentTimeMillis() - start < FUNC_RUN_MILLIS) {}
+    }
+
+}