fatal handler: don't let a concurrent fatal mask the first one - #22671
Merged
stelfrag merged 3 commits intoJun 11, 2026
Merged
Conversation
Copy link
Copy Markdown
Collaborator
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
1 issue found across 1 file
Confidence score: 3/5
- In src/libnetdata/log/nd_log.c, the concurrent fatal path can still hang inside fprintf/fflush(stderr), so a process that should hit the bounded wait and _exit(1) may instead block indefinitely during a crash, delaying recovery or restart. Before merging, make the fatal-path emission non-blocking (or bypass stdio entirely) so _exit(1) remains guaranteed under contention.
sequenceDiagram
participant T1 as Thread A
participant T2 as Thread B
participant Fatal as netdata_logger_fatal()
participant Stderr as stderr
participant Exit as Process Exit
Note over T1,Exit: Fatal handler - thread-safe concurrency control
T1->>Fatal: call fatal()
Fatal->>Fatal: check this_thread_in_fatal flag
alt Same-thread re-entry (this_thread_in_fatal true)
Fatal->>Stderr: log "RECURSIVE FATAL" message
Fatal->>Fatal: call recursive_fatal_abort()
Fatal-->>Exit: abort process (crash)
else First entry for this thread
Fatal->>Fatal: set this_thread_in_fatal = true
Fatal->>Fatal: atomic increment threads_in_fatal
alt Threads_in_fatal > 1 (concurrent fatal from other thread)
Fatal->>Stderr: log "CONCURRENT FATAL" message
Fatal->>Fatal: sleep(2) - wait for first fatal to finish
Fatal->>Exit: _exit(1) - quiet exit, no abort
else First thread in fatal (threads_in_fatal == 1)
Fatal->>Fatal: proceed with logging fatal event
Note over Fatal: Write fatal info to daemon_status_file
Fatal->>Fatal: call fatal_abort_internal_checks()
Fatal-->>Exit: abort process with first crash
end
end
Note over T1,T2: Concurrent scenario - second thread arrives
T2->>Fatal: call fatal() while T1 already in fatal
Fatal->>Fatal: check this_thread_in_fatal flag (false for T2)
Fatal->>Fatal: set this_thread_in_fatal = true (T2)
Fatal->>Fatal: atomic increment threads_in_fatal (now 2)
alt threads_in_fatal > 1
Fatal->>Stderr: log "CONCURRENT FATAL" for T2
Fatal->>Fatal: sleep(2) - wait for T1 to finish
Fatal->>Exit: _exit(1) - T2 exits quietly
end
Note over T1: Meanwhile T1 continues
T1->>Fatal: complete fatal handling
T1->>Exit: abort process with first crash details
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Sorry, something went wrong.
Comment thread
src/libnetdata/log/nd_log.c
Outdated
…deadlocks on `stderr` lock during multi-threaded fatal scenarios.
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Sorry, something went wrong.
Comment thread
src/libnetdata/log/nd_log.c
Outdated
Copy link
Copy Markdown
|
Sorry, something went wrong.
stelfrag
marked this pull request as ready for review
June 11, 2026 10:23
thiagoftsm
approved these changes
Jun 11, 2026
thiagoftsm
left a comment
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
No issues happening after hours running. LGTM!
Sorry, something went wrong.
Merged
Ferroin
pushed a commit
that referenced
this pull request
Jul 15, 2026
* Introduce thread-local and global re-entrancy safeguards in `netdata_logger_fatal()` * Replace `stdio` usage with `write()` in fatal logging paths to avoid deadlocks on `stderr` lock during multi-threaded fatal scenarios. * Fix off-by-one error in `write()` size calculation for fatal log messages (cherry picked from commit cce16fe)
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
Reviewers
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.
Summary
netdata_logger_fatal()'s recursion guard used a single process-wide counter, so it couldn't tell apart two different situations:
Make the guard thread-aware:
Summary by cubic
Make the fatal handler thread-aware so a concurrent fatal can't mask the first crash. Same-thread re-entry still aborts; other threads log once, wait briefly, then exit to avoid a second crash and stderr deadlocks.
Written for commit b6d3d19. Summary will update on new commits.