From f3db26edb49b85c65e81f3586647e696ef918d90 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Thu, 5 Feb 2026 17:44:50 +0100 Subject: [PATCH 1/3] Fix musl TLS init crash with signal blocking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Block profiling signals during context_tls_v1 initialization to prevent signal handler reentrancy into musl's TLS setup. Consolidate CriticalSection and SignalBlocker into guards module. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 --- ddprof-lib/src/main/cpp/callTraceStorage.cpp | 2 +- ddprof-lib/src/main/cpp/context.cpp | 12 ++- ddprof-lib/src/main/cpp/ctimer.h | 3 + ddprof-lib/src/main/cpp/ctimer_linux.cpp | 2 +- .../cpp/{criticalSection.cpp => guards.cpp} | 23 ++++- .../main/cpp/{criticalSection.h => guards.h} | 97 ++++++++++++++++--- ddprof-lib/src/main/cpp/itimer.cpp | 2 +- ddprof-lib/src/main/cpp/perfEvents_linux.cpp | 2 +- ddprof-lib/src/main/cpp/profiler.cpp | 2 +- ddprof-lib/src/main/cpp/wallClock.cpp | 2 +- .../src/test/cpp/stress_callTraceStorage.cpp | 2 +- 11 files changed, 124 insertions(+), 25 deletions(-) rename ddprof-lib/src/main/cpp/{criticalSection.cpp => guards.cpp} (66%) rename ddprof-lib/src/main/cpp/{criticalSection.h => guards.h} (50%) diff --git a/ddprof-lib/src/main/cpp/callTraceStorage.cpp b/ddprof-lib/src/main/cpp/callTraceStorage.cpp index 311058d48..00f5ea352 100644 --- a/ddprof-lib/src/main/cpp/callTraceStorage.cpp +++ b/ddprof-lib/src/main/cpp/callTraceStorage.cpp @@ -11,7 +11,7 @@ #include "thread.h" #include "vmEntry.h" // For BCI_ERROR constant #include "arch.h" // For LP64_ONLY macro and COMMA macro -#include "criticalSection.h" // For table swap critical sections +#include "guards.h" // For table swap critical sections #include "primeProbing.h" #include "thread.h" #include diff --git a/ddprof-lib/src/main/cpp/context.cpp b/ddprof-lib/src/main/cpp/context.cpp index 7f63fd453..3348c7e0f 100644 --- a/ddprof-lib/src/main/cpp/context.cpp +++ b/ddprof-lib/src/main/cpp/context.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2021, 2022 Datadog, Inc + * Copyright 2021, 2026 Datadog, Inc * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,16 +17,24 @@ #include "context.h" #include "counters.h" #include "os.h" +#include "guards.h" #include "thread.h" #include DLLEXPORT thread_local Context context_tls_v1; Context& Contexts::initializeContextTls() { + // Block profiling signals during context_tls_v1 first access to prevent + // signal handler from interrupting musl's TLS initialization + SignalBlocker blocker; + + // FIRST access to context_tls_v1 - triggers musl TLS init on first call // ProfiledThread::current() will never return nullptr Context& ctx = context_tls_v1; - // Store pointer for signal-safe access + // Store pointer in ProfiledThread for signal-safe access ProfiledThread::current()->markContextTlsInitialized(&ctx); + + // Signal mask automatically restored when blocker goes out of scope return ctx; } diff --git a/ddprof-lib/src/main/cpp/ctimer.h b/ddprof-lib/src/main/cpp/ctimer.h index 3fc01289d..0f5340b2d 100644 --- a/ddprof-lib/src/main/cpp/ctimer.h +++ b/ddprof-lib/src/main/cpp/ctimer.h @@ -55,6 +55,9 @@ class CTimer : public Engine { inline void enableEvents(bool enabled) { __atomic_store_n(&_enabled, enabled, __ATOMIC_RELEASE); } + + // Get the signal number used by CTimer (0 if not initialized) + static int getSignal() { return _signal; } }; #else diff --git a/ddprof-lib/src/main/cpp/ctimer_linux.cpp b/ddprof-lib/src/main/cpp/ctimer_linux.cpp index e15862faf..444fe662a 100644 --- a/ddprof-lib/src/main/cpp/ctimer_linux.cpp +++ b/ddprof-lib/src/main/cpp/ctimer_linux.cpp @@ -17,7 +17,7 @@ #ifdef __linux__ -#include "criticalSection.h" +#include "guards.h" #include "ctimer.h" #include "debugSupport.h" #include "libraries.h" diff --git a/ddprof-lib/src/main/cpp/criticalSection.cpp b/ddprof-lib/src/main/cpp/guards.cpp similarity index 66% rename from ddprof-lib/src/main/cpp/criticalSection.cpp rename to ddprof-lib/src/main/cpp/guards.cpp index f4e944f00..23850349e 100644 --- a/ddprof-lib/src/main/cpp/criticalSection.cpp +++ b/ddprof-lib/src/main/cpp/guards.cpp @@ -1,9 +1,20 @@ /* - * Copyright 2025, Datadog, Inc. - * SPDX-License-Identifier: Apache-2.0 + * Copyright 2025, 2026 Datadog, Inc + * + * 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. */ -#include "criticalSection.h" +#include "guards.h" #include "common.h" #include "os.h" #include "thread.h" @@ -27,7 +38,8 @@ CriticalSection::CriticalSection() : _entered(false), _using_fallback(false), _w uint32_t bit_index = tid % 64; _bit_mask = 1ULL << bit_index; - uint64_t old_word = __atomic_fetch_or(&_fallback_bitmap[_word_index], _bit_mask, __ATOMIC_RELAXED); + // Use ACQUIRE ordering to ensure visibility of protected data after acquiring critical section + uint64_t old_word = __atomic_fetch_or(&_fallback_bitmap[_word_index], _bit_mask, __ATOMIC_ACQUIRE); _entered = !(old_word & _bit_mask); // Success if bit was previously 0 } } @@ -36,7 +48,8 @@ CriticalSection::~CriticalSection() { if (_entered) { if (_using_fallback) { // Clear the bit atomically for fallback bitmap - __atomic_fetch_and(&_fallback_bitmap[_word_index], ~_bit_mask, __ATOMIC_RELAXED); + // Use RELEASE ordering to ensure protected data writes are visible before releasing + __atomic_fetch_and(&_fallback_bitmap[_word_index], ~_bit_mask, __ATOMIC_RELEASE); } else { // Release ProfiledThread flag ProfiledThread* current = ProfiledThread::currentSignalSafe(); diff --git a/ddprof-lib/src/main/cpp/criticalSection.h b/ddprof-lib/src/main/cpp/guards.h similarity index 50% rename from ddprof-lib/src/main/cpp/criticalSection.h rename to ddprof-lib/src/main/cpp/guards.h index 307e44c45..05d7eb31d 100644 --- a/ddprof-lib/src/main/cpp/criticalSection.h +++ b/ddprof-lib/src/main/cpp/guards.h @@ -1,23 +1,36 @@ /* - * Copyright 2025, Datadog, Inc. - * SPDX-License-Identifier: Apache-2.0 + * Copyright 2025, 2026 Datadog, Inc + * + * 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. */ -#ifndef _CRITICALSECTION_H -#define _CRITICALSECTION_H +#ifndef _GUARDS_H +#define _GUARDS_H #include #include +#include +#include /** * Race-free critical section using atomic compare-and-swap. - * + * * Hybrid implementation: * - Primary: Uses ProfiledThread storage when available (zero memory overhead) * - Fallback: Hash-based bitmap for stress tests and cases without ProfiledThread * * This approach is async-signal-safe and avoids TLS allocation issues. - * + * * Usage: * { * CriticalSection cs; // Atomically claim critical section @@ -25,7 +38,7 @@ * // Complex data structure operations * // Signal handlers will be blocked from entering * } // Critical section automatically released - * + * * This eliminates race conditions between signal handlers and normal code * by ensuring only one can hold the critical section at a time per thread. * @@ -54,17 +67,17 @@ class CriticalSection { bool _using_fallback; // Track which storage mechanism we're using uint32_t _word_index; // For fallback bitmap cleanup uint64_t _bit_mask; // For fallback bitmap cleanup - + public: CriticalSection(); ~CriticalSection(); - + // Non-copyable, non-movable CriticalSection(const CriticalSection&) = delete; CriticalSection& operator=(const CriticalSection&) = delete; CriticalSection(CriticalSection&&) = delete; CriticalSection& operator=(CriticalSection&&) = delete; - + // Check if this instance successfully entered the critical section bool entered() const { return _entered; } @@ -73,4 +86,66 @@ class CriticalSection { static uint32_t hash_tid(int tid); }; -#endif // _CRITICALSECTION_H +/** + * RAII guard to block profiling signals during critical operations. + * + * Blocks SIGPROF and SIGVTALRM signals on construction and automatically + * restores the original signal mask on destruction. This prevents signal + * handlers from interrupting operations that are not async-signal-safe, + * such as musl libc's TLS initialization. + * + * !WARNING! + * For guarding access to code running as a signal handler use CriticalSection + * !WARNING! + * + * Usage: + * { + * SignalBlocker blocker; // Blocks profiling signals + * // Perform operations that must not be interrupted by signals + * // (e.g., TLS initialization, malloc, etc.) + * } // Signal mask automatically restored + * + * The blocker is exception-safe: the signal mask will be restored even + * if an exception is thrown within the protected scope. + * + * Note: This only blocks signals for the current thread. Other threads + * continue to receive profiling signals normally. + */ +class SignalBlocker { +private: + sigset_t _old_mask; + bool _active; + +public: + SignalBlocker() : _active(false) { + sigset_t prof_signals; + sigemptyset(&prof_signals); + + // Add all profiling signals that could interrupt us + sigaddset(&prof_signals, SIGPROF); // Used by ITimer and CTimer + sigaddset(&prof_signals, SIGVTALRM); // Used by WallClock +#ifdef __linux__ + // Block RT signals used by TLS priming and potentially other profilers (Linux-only) + // This prevents any RT signal handler from interrupting TLS initialization + for (int sig = SIGRTMIN; sig <= SIGRTMIN + 5 && sig <= SIGRTMAX; sig++) { + sigaddset(&prof_signals, sig); + } +#endif + + if (pthread_sigmask(SIG_BLOCK, &prof_signals, &_old_mask) == 0) { + _active = true; + } + } + + ~SignalBlocker() { + if (_active) { + pthread_sigmask(SIG_SETMASK, &_old_mask, nullptr); + } + } + + // Non-copyable + SignalBlocker(const SignalBlocker&) = delete; + SignalBlocker& operator=(const SignalBlocker&) = delete; +}; + +#endif // _GUARDS_H diff --git a/ddprof-lib/src/main/cpp/itimer.cpp b/ddprof-lib/src/main/cpp/itimer.cpp index 77c8cee02..01f8d8c59 100644 --- a/ddprof-lib/src/main/cpp/itimer.cpp +++ b/ddprof-lib/src/main/cpp/itimer.cpp @@ -23,7 +23,7 @@ #include "thread.h" #include "threadState.inline.h" #include "vmStructs.h" -#include "criticalSection.h" +#include "guards.h" #include volatile bool ITimer::_enabled = false; diff --git a/ddprof-lib/src/main/cpp/perfEvents_linux.cpp b/ddprof-lib/src/main/cpp/perfEvents_linux.cpp index 6cb2b3bad..e311f5db7 100644 --- a/ddprof-lib/src/main/cpp/perfEvents_linux.cpp +++ b/ddprof-lib/src/main/cpp/perfEvents_linux.cpp @@ -20,7 +20,7 @@ #include "arch.h" #include "arguments.h" #include "context.h" -#include "criticalSection.h" +#include "guards.h" #include "debugSupport.h" #include "libraries.h" #include "log.h" diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 2a525fe6a..b44edfcc2 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -7,7 +7,7 @@ #include "profiler.h" #include "asyncSampleMutex.h" #include "context.h" -#include "criticalSection.h" +#include "guards.h" #include "common.h" #include "counters.h" #include "ctimer.h" diff --git a/ddprof-lib/src/main/cpp/wallClock.cpp b/ddprof-lib/src/main/cpp/wallClock.cpp index 388b9deb4..b9effb989 100644 --- a/ddprof-lib/src/main/cpp/wallClock.cpp +++ b/ddprof-lib/src/main/cpp/wallClock.cpp @@ -15,7 +15,7 @@ #include "thread.h" #include "threadState.inline.h" #include "vmStructs.h" -#include "criticalSection.h" +#include "guards.h" #include #include #include // For std::sort and std::binary_search diff --git a/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp b/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp index 1557b0e35..f11d0c10d 100644 --- a/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp +++ b/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp @@ -6,7 +6,7 @@ #include "gtest/gtest.h" #include "callTraceStorage.h" #include "callTraceHashTable.h" -#include "criticalSection.h" +#include "guards.h" #include #include #include From ff1e36f64ce25db8239ec2edd72c85f7e78d00c6 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 6 Feb 2026 11:00:53 +0100 Subject: [PATCH 2/3] Fix CI test summary regex to handle reusable workflow job names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .github/scripts/generate-test-summary.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/scripts/generate-test-summary.sh b/.github/scripts/generate-test-summary.sh index 0037391f9..f652d6a0e 100755 --- a/.github/scripts/generate-test-summary.sh +++ b/.github/scripts/generate-test-summary.sh @@ -91,7 +91,9 @@ while IFS= read -r job; do # Only process test jobs (match pattern: test-linux-{libc}-{arch} ({java}, {config})) # Note: regex stored in variable to avoid bash parsing issues with ) character - test_job_pattern='^test-linux-([a-z]+)-([a-z0-9]+) \(([^,]+), ([^)]+)\)$' + # Note: No ^ anchor because reusable workflow jobs are prefixed with caller job name + # e.g., "test-matrix / test-linux-glibc-amd64 (8, debug)" + test_job_pattern='test-linux-([a-z]+)-([a-z0-9]+) \(([^,]+), ([^)]+)\)$' if [[ "$name" =~ $test_job_pattern ]]; then libc="${BASH_REMATCH[1]}" arch="${BASH_REMATCH[2]}" From 879d6a303a98156002449da289499a7095f640c2 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 6 Feb 2026 11:55:15 +0100 Subject: [PATCH 3/3] Fix bash arithmetic and printf issues in test summary script MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .github/scripts/generate-test-summary.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/scripts/generate-test-summary.sh b/.github/scripts/generate-test-summary.sh index f652d6a0e..8404e3d87 100755 --- a/.github/scripts/generate-test-summary.sh +++ b/.github/scripts/generate-test-summary.sh @@ -160,10 +160,10 @@ for key in "${!job_status[@]}"; do dur="${job_duration[$key]:-0}" case "$status" in - success) ((passed_jobs++)) ;; + success) ((++passed_jobs)) ;; failure) ;; # already counted - skipped) ((skipped_jobs++)) ;; - cancelled) ((cancelled_jobs++)) ;; + skipped) ((++skipped_jobs)) ;; + cancelled) ((++cancelled_jobs)) ;; esac ((total_duration += dur)) || true @@ -257,9 +257,9 @@ log "Generating markdown summary..." echo "" # Separator row - printf "|----------|" + printf "%s" "|----------|" for _ in "${sorted_java[@]}"; do - printf "--------|" + printf "%s" "--------|" done echo ""