fix: make ProfilerSignalManager::_handler atomic to prevent data race#8451
Open
korniltsev-grafanista wants to merge 1 commit intoDataDog:masterfrom
Open
Conversation
gleocadie
approved these changes
Apr 14, 2026
Collaborator
gleocadie
left a comment
gleocadie
left a comment
There was a problem hiding this comment.
@korniltsev-grafanista your commit must be signed for your PR to be mergeable.
if not possible for you, I can do something
| } | ||
| std::unique_lock<std::mutex> lock(_handlerRegisterMutex); | ||
|
|
||
| current = _handler.load(std::memory_order_acquire); |
Collaborator
One piece of information I was not aware, as of today for security reasons (CI cannot run on external forks), I'll create the branch, cherry-pick your commit and create a PR with it. |
_handler is written on a normal thread in RegisterHandler and read inside a signal handler in CallCustomHandler. Without atomics this is undefined behavior — the C++ standard requires lock-free atomics for signal-safe access ([support.signal]/3). Also fixes the double-checked locking in RegisterHandler: the second check after acquiring the mutex was comparing a stale local copy instead of re-loading from the (now atomic) _handler. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
korniltsev-grafanista
force-pushed
the
kk/fix-signal-handler-atomic-dd
branch
from
63696f0 to
3928517
Compare
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
_handleris a plain function pointer that is written on a normal thread inRegisterHandlerand read inside a signal handler inCallCustomHandler. Without atomics this is a data race — undefined behavior in C++. The C++ standard ([support.signal]/3) requires lock-free atomics for signal-safe access, andstd::atomic<T*>is lock-free on all relevant platforms.Changes:
_handlerasstd::atomic<HandlerFn_t>instead of plainHandlerFn_tload/storewith appropriate memory ordering at every access siteRegisterHandler: the second check after acquiring the mutex was comparing a stale local copy instead of re-loading_handlerNote:
_isHandlerInPlacehas a similar (less critical) data race, left for a follow-up.Co-Reviewed-By: Claude Opus 4.6 (1M context)