fix(profiler): eliminate SIGSEGV race on _class_map.clear() in signal handler and dump path

ddprof-lib/src/main/cpp/flightRecorder.cpp

@@ -1153,9 +1153,9 @@ void Recording::writeCpool(Buffer *buf) {
  buf->put8(12);
 
  // classMap() is shared across the dump (this thread) and the JVMTI shared-lock
- // writers (Profiler::lookupClass and friends). writeClasses() takes
- // classMapLock() shared while reading; the exclusive classMap()->clear() in
- // Profiler::dump runs only after this method returns.
+ // writers (Profiler::lookupClass and friends). writeClasses() holds
+ // classMapSharedGuard() for its full duration; the exclusive classMap()->clear()
+ // in Profiler::dump runs only after this method returns.
  Lookup lookup(this, &_method_map, Profiler::instance()->classMap());
  writeFrameTypes(buf);
  writeThreadStates(buf);
@@ -1369,15 +1369,12 @@ void Recording::writeMethods(Buffer *buf, Lookup *lookup) {
 
 void Recording::writeClasses(Buffer *buf, Lookup *lookup) {
  std::map<u32, const char *> classes;
- // Hold classMapLock() shared while reading. JVMTI writers
- // (Profiler::lookupClass, ObjectSampler, LivenessTracker) also take it
- // shared and use CAS-based inserts that are safe against concurrent shared
- // readers; the exclusive classMap()->clear() in Profiler::dump runs only
- // after writeClasses() returns and is blocked here.
- {
- SharedLockGuard guard(Profiler::instance()->classMapLock());
- lookup->_classes->collect(classes);
- }
+ // Hold classMapSharedGuard() for the full function. The const char* pointers
+ // stored in classes point into dictionary row storage; clear() frees that
+ // storage under the exclusive lock, so we must not release the shared lock
+ // until we have finished iterating.
+ auto guard = Profiler::instance()->classMapSharedGuard();
+ lookup->_classes->collect(classes);
 
  buf->putVar64(T_CLASS);
  buf->putVar64(classes.size());

ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp

@@ -540,7 +540,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex
  // clear() concurrently frees. Take the lock shared via
  // try-lock; if an exclusive clear() is in progress, drop
  // the synthetic frame rather than read freed memory.
-OptionalSharedLockGuard guard(profiler->classMapLock());
+auto guard = profiler->classMapTrySharedGuard();
  if (guard.ownsLock()) {
  u32 class_id = profiler->classMap()->bounded_lookup(
  symbol->body(), symbol->length(), 0);

ddprof-lib/src/main/cpp/profiler.h

@@ -204,7 +204,8 @@ class alignas(alignof(SpinLock)) Profiler {
  Engine *wallEngine() { return _wall_engine; }
 
  Dictionary *classMap() { return &_class_map; }
- SpinLock *classMapLock() { return &_class_map_lock; }
+ SharedLockGuard classMapSharedGuard() { return SharedLockGuard(&_class_map_lock); }
+ BoundedOptionalSharedLockGuard classMapTrySharedGuard() { return BoundedOptionalSharedLockGuard(&_class_map_lock); }
  Dictionary *stringLabelMap() { return &_string_label_map; }
  Dictionary *contextValueMap() { return &_context_value_map; }
  u32 numContextAttributes() { return _num_context_attributes; }

ddprof-lib/src/main/cpp/spinLock.h

@@ -47,11 +47,30 @@ class alignas(DEFAULT_CACHE_LINE_SIZE) SpinLock {
  void unlock() { __sync_fetch_and_sub(&_lock, 1); }
 
  bool tryLockShared() {
+ // Spins while no exclusive lock is held and the CAS to acquire a shared
+ // lock fails (due to concurrent reader contention). Returns false ONLY when
+ // an exclusive lock is observed (_lock > 0); never returns false spuriously.
  int value;
  while ((value = __atomic_load_n(&_lock, __ATOMIC_ACQUIRE)) <= 0) {
  if (__sync_bool_compare_and_swap(&_lock, value, value - 1)) {
  return true;
  }
+ spinPause();
+ }
+ return false;
+ }
+
+ // Signal-safe variant: returns false after at most 5 CAS attempts.
+ // Use only in signal-handler paths where spinning indefinitely is unsafe.
+ bool tryLockSharedBounded() {
+ for (int attempts = 0; attempts < 5; ++attempts) {
+ int value = __atomic_load_n(&_lock, __ATOMIC_ACQUIRE);
+ if (value > 0) {
+ return false;
+ }
+ if (__sync_bool_compare_and_swap(&_lock, value, value - 1)) {
+ return true;
+ }
  }
  return false;
  }
@@ -88,9 +107,9 @@ class SharedLockGuard {
 class OptionalSharedLockGuard {
  SpinLock* _lock;
 public:
- OptionalSharedLockGuard(SpinLock* lock) : _lock(lock) {
+explicit OptionalSharedLockGuard(SpinLock* lock) : _lock(lock) {
  if (!_lock->tryLockShared()) {
- // Locking failed, no need to unlock.
+ // Exclusive lock is held; no unlock needed. Only fails when an exclusive lock is observed.
  _lock = nullptr;
  }
  }
@@ -108,6 +127,33 @@ class OptionalSharedLockGuard {
  OptionalSharedLockGuard& operator=(OptionalSharedLockGuard&&) = delete;
 };
 
+// Signal-safe variant of OptionalSharedLockGuard: uses bounded CAS retries
+// and may fail spuriously when racing other shared lockers. Use ONLY in
+// signal-handler paths where spinning indefinitely is unsafe; do NOT use
+// in hot recording paths where silent acquisition failures would drop events.
+class BoundedOptionalSharedLockGuard {
+ SpinLock* _lock;
+public:
+ explicit BoundedOptionalSharedLockGuard(SpinLock* lock) : _lock(lock) {
+ if (!_lock->tryLockSharedBounded()) {
+ // Locking failed (bounded retries exhausted or exclusive lock held); no unlock needed.
+ _lock = nullptr;
+ }
+ }
+ ~BoundedOptionalSharedLockGuard() {
+ if (_lock != nullptr) {
+ _lock->unlockShared();
+ }
+ }
+ bool ownsLock() { return _lock != nullptr; }
+
+ // Non-copyable and non-movable
+ BoundedOptionalSharedLockGuard(const BoundedOptionalSharedLockGuard&) = delete;
+ BoundedOptionalSharedLockGuard& operator=(const BoundedOptionalSharedLockGuard&) = delete;
+ BoundedOptionalSharedLockGuard(BoundedOptionalSharedLockGuard&&) = delete;
+ BoundedOptionalSharedLockGuard& operator=(BoundedOptionalSharedLockGuard&&) = delete;
+};
+
 class ExclusiveLockGuard {
 private:
  SpinLock* _lock;

ddprof-lib/src/test/cpp/dictionary_concurrent_ut.cpp

@@ -199,8 +199,7 @@ TEST(DictionaryConcurrent, SignalHandlerBoundedLookupVsDumpClear) {
  std::atomic<long> total_clears{0};
 
  // Simulate walkVM signal-handler threads: tryLockShared → bounded_lookup(0)
- // → unlockShared. Mirrors the OptionalSharedLockGuard + ownsLock() guard in
- // hotspotSupport.cpp.
+ // → unlockShared. Mirrors the OptionalSharedLockGuard + ownsLock() guard.
  std::vector<std::thread> signal_threads;
  signal_threads.reserve(kSignalThreads);
  for (int w = 0; w < kSignalThreads; ++w) {
@@ -250,3 +249,74 @@ TEST(DictionaryConcurrent, SignalHandlerBoundedLookupVsDumpClear) {
  EXPECT_GT(total_reads.load() + total_skips.load(), 0L);
  EXPECT_GT(total_clears.load(), 0L);
 }
+
+// (4) Same race as (3) but using BoundedOptionalSharedLockGuard, which is the
+// guard classMapTrySharedGuard() now returns in hotspotSupport.cpp. The bounded
+// variant may fail spuriously under reader pressure (≤5 CAS attempts); this
+// verifies it still prevents use-after-free without deadlocking.
+TEST(DictionaryConcurrent, SignalHandlerBoundedOptionalLookupVsDumpClear) {
+ Dictionary dict(/*id=*/0);
+ SpinLock lock;
+
+ constexpr int kPreload = 64;
+ for (int i = 0; i < kPreload; ++i) {
+ char buf[64];
+ snprintf(buf, sizeof(buf), "Lcom/example/Preloaded%d;", i);
+ lock.lock();
+ dict.lookup(buf, strlen(buf));
+ lock.unlock();
+ }
+
+ constexpr int kSignalThreads = 4;
+ const auto kDuration = std::chrono::milliseconds(500);
+
+ std::atomic<bool> stop{false};
+ std::atomic<long> total_reads{0};
+ std::atomic<long> total_skips{0};
+ std::atomic<long> total_clears{0};
+
+ // Simulate hotspotSupport.cpp: BoundedOptionalSharedLockGuard + bounded_lookup(0).
+ std::vector<std::thread> signal_threads;
+ signal_threads.reserve(kSignalThreads);
+ for (int w = 0; w < kSignalThreads; ++w) {
+ signal_threads.emplace_back([&, w]() {
+ char buf[64];
+ int counter = 0;
+ while (!stop.load(std::memory_order_relaxed)) {
+ snprintf(buf, sizeof(buf), "Lcom/example/Preloaded%d;",
+ counter % kPreload);
+ size_t len = strlen(buf);
+ BoundedOptionalSharedLockGuard guard(&lock);
+ if (guard.ownsLock()) {
+ unsigned int id = dict.bounded_lookup(buf, len, 0);
+ (void)id;
+ total_reads.fetch_add(1, std::memory_order_relaxed);
+ } else {
+ total_skips.fetch_add(1, std::memory_order_relaxed);
+ }
+ ++counter;
+ }
+ });
+ }
+
+ std::thread dump_thread([&]() {
+ while (!stop.load(std::memory_order_relaxed)) {
+ std::this_thread::sleep_for(std::chrono::milliseconds(10));
+ lock.lock();
+ dict.clear();
+ lock.unlock();
+ total_clears.fetch_add(1, std::memory_order_relaxed);
+ }
+ });
+
+ std::this_thread::sleep_for(kDuration);
+ stop.store(true, std::memory_order_relaxed);
+
+ for (auto& t : signal_threads) {
+ t.join();
+ }
+ dump_thread.join();
+
+ EXPECT_GT(total_reads.load() + total_skips.load(), 0L);
+ EXPECT_GT(total_clears.load(), 0L);
+}

ddprof-lib/src/test/cpp/spinlock_bounded_ut.cpp

@@ -0,0 +1,135 @@
+/*
+ * Copyright 2026, Datadog, Inc.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <gtest/gtest.h>
+#include <atomic>
+#include <chrono>
+#include <thread>
+#include <vector>
+
+#include "spinLock.h"
+#include "../../main/cpp/gtest_crash_handler.h"
+
+// Unit tests for tryLockSharedBounded() and BoundedOptionalSharedLockGuard —
+// the signal-safe locking API introduced to replace unbounded spinning in
+// hotspotSupport.cpp (classMapTrySharedGuard()).
+
+static constexpr char SPINLOCK_BOUNDED_TEST_NAME[] = "SpinLockBounded";
+
+namespace {
+
+class SpinLockBoundedSetup {
+public:
+ SpinLockBoundedSetup() {
+ installGtestCrashHandler<SPINLOCK_BOUNDED_TEST_NAME>();
+ }
+ ~SpinLockBoundedSetup() {
+ restoreDefaultSignalHandlers();
+ }
+};
+
+static SpinLockBoundedSetup spinlock_bounded_global_setup;
+
+} // namespace
+
+TEST(SpinLockBounded, BoundedTryLockSucceedsOnFreeLock) {
+ SpinLock lock;
+ EXPECT_TRUE(lock.tryLockSharedBounded());
+ lock.unlockShared();
+}
+
+TEST(SpinLockBounded, BoundedTryLockFailsWhenExclusiveLockHeld) {
+ SpinLock lock;
+ lock.lock();
+ EXPECT_FALSE(lock.tryLockSharedBounded());
+ lock.unlock();
+}
+
+// Multiple shared holders must coexist — bounded acquire must not treat a
+// concurrent reader's negative _lock value as an exclusive lock.
+TEST(SpinLockBounded, BoundedTryLockAllowsMultipleSharedHolders) {
+ SpinLock lock;
+ ASSERT_TRUE(lock.tryLockSharedBounded());
+ EXPECT_TRUE(lock.tryLockSharedBounded());
+ lock.unlockShared();
+ lock.unlockShared();
+}
+
+TEST(SpinLockBounded, BoundedGuardOwnsLockWhenFree) {
+ SpinLock lock;
+ BoundedOptionalSharedLockGuard guard(&lock);
+ EXPECT_TRUE(guard.ownsLock());
+}
+
+// When the exclusive lock is held the guard must not own the lock, and its
+// destructor must not accidentally release the exclusive lock.
+TEST(SpinLockBounded, BoundedGuardFailsWhenExclusiveLockHeld) {
+ SpinLock lock;
+ lock.lock();
+ {
+ BoundedOptionalSharedLockGuard guard(&lock);
+ EXPECT_FALSE(guard.ownsLock());
+ }
+ // Exclusive lock must still be held — unlock() must succeed exactly once.
+ lock.unlock();
+}
+
+// After the guard goes out of scope the lock must be fully released so that an
+// exclusive acquire succeeds.
+TEST(SpinLockBounded, BoundedGuardReleasesLockOnDestruction) {
+ SpinLock lock;
+ {
+ BoundedOptionalSharedLockGuard guard(&lock);
+ ASSERT_TRUE(guard.ownsLock());
+ }
+ EXPECT_TRUE(lock.tryLock());
+ lock.unlock();
+}
+
+// Stress: concurrent BoundedOptionalSharedLockGuard readers vs an exclusive
+// locker. Mirrors the classMapTrySharedGuard() signal-handler path vs the
+// Profiler::dump path, using the RAII type that hotspotSupport.cpp now uses.
+TEST(SpinLockBounded, BoundedGuardConcurrentWithExclusiveLock) {
+ SpinLock lock;
+ constexpr int kReaders = 4;
+ const auto kDuration = std::chrono::milliseconds(300);
+
+ std::atomic<bool> stop{false};
+ std::atomic<long> total_acquired{0};
+ std::atomic<long> total_skipped{0};
+ std::atomic<long> total_exclusive{0};
+
+ std::vector<std::thread> readers;
+ readers.reserve(kReaders);
+ for (int i = 0; i < kReaders; ++i) {
+ readers.emplace_back([&]() {
+ while (!stop.load(std::memory_order_relaxed)) {
+ BoundedOptionalSharedLockGuard guard(&lock);
+ if (guard.ownsLock()) {
+ total_acquired.fetch_add(1, std::memory_order_relaxed);
+ } else {
+ total_skipped.fetch_add(1, std::memory_order_relaxed);
+ }
+ }
+ });
+ }
+
+ std::thread exclusive_thread([&]() {
+ while (!stop.load(std::memory_order_relaxed)) {
+ std::this_thread::sleep_for(std::chrono::milliseconds(5));
+ lock.lock();
+ lock.unlock();
+ total_exclusive.fetch_add(1, std::memory_order_relaxed);
+ }
+ });
+
+ std::this_thread::sleep_for(kDuration);
+ stop.store(true, std::memory_order_relaxed);
+ for (auto& t : readers) t.join();
+ exclusive_thread.join();
+
+ EXPECT_GT(total_acquired.load(), 0L);
+ EXPECT_GT(total_exclusive.load(), 0L);
+}