Fix memory-safety and correctness bugs surfaced by Coverity audit (part 8) - #22293
Merged
stelfrag merged 1 commit intoApr 27, 2026
Merged
Fix memory-safety and correctness bugs surfaced by Coverity audit (part 8)#22293stelfrag merged 1 commit into
stelfrag merged 1 commit into
Conversation
Copy link
Copy Markdown
Member
Sorry, something went wrong.
Coverity CID 442107 (SLEEP): remove-stale-node held the global RRD write lock while freeing a host. Unlink the host under the lock, then run teardown/freeing outside the lock so logging and stream shutdown waits do not block global RRD progress.
4 tasks
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
No issues found across 3 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
sequenceDiagram
participant D as Daemon (Commands)
participant L as RRD Global Lock
participant H as RRDHost Logic
participant I as RRD Index & List
participant S as Health & Collection Subsystems
Note over D,S: Ephemeral Host Removal Flow
D->>H: NEW: rrdhost_free___without_having_rrd_wrlock(host)
H->>L: rrd_wrlock()
Note right of L: Blocks other writers
H->>I: NEW: rrdhost_unlink___while_having_rrd_wrlock()
I-->>H: Removed from GUID index & Linked List
H->>L: rrd_wrunlock()
Note right of L: Lock released early
Note over H,S: CHANGED: Slow teardown occurs outside global lock
H->>S: rrdhost_cleanup_data_collection_and_health()
activate S
Note right of S: Blocking I/O, stream shutdown, health cleanup
S-->>H: Teardown complete
deactivate S
H->>H: freez(host)
H-->>D: Return success status
Sorry, something went wrong.
Copy link
Copy Markdown
|
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
stelfrag
approved these changes
Apr 27, 2026
Merged
Ferroin
pushed a commit
that referenced
this pull request
Jul 15, 2026
…rt 8) (#22293) daemon: free removed stale nodes outside rrd lock Coverity CID 442107 (SLEEP): remove-stale-node held the global RRD write lock while freeing a host. Unlink the host under the lock, then run teardown/freeing outside the lock so logging and stream shutdown waits do not block global RRD progress. (cherry picked from commit 8e49531)
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
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
Single follow-up commit not yet covered by parts 1-7 (#22266, #22267, #22268, #22270, #22279, #22280, #22281).
@stelfrag — could you pick this one up and review when you get a chance? Thanks for splitting the original PR into reviewable parts.
CID 442107 (SLEEP) — daemon: free removed stale nodes outside rrd lock
remove_ephemeral_host() in src/daemon/commands.c held the global RRD write lock while calling rrdhost_free___while_having_rrd_wrlock(), which performs blocking teardown work (logging, stream-shutdown waits, data collection cleanup, health cleanup). Every other RRD writer was stalled meanwhile.
The fix splits host removal into:
A new wrapper rrdhost_free___without_having_rrd_wrlock() encapsulates the lock-then-unlink-then-unlock-then-free pattern for callers like remove_ephemeral_host().
Files changed:
3 files changed, +23 -15.
Test plan
Summary by cubic
Fixes Coverity CID 442107 by moving slow host teardown outside the global RRD write lock to prevent stalls. Host unlinking now happens under the lock, with teardown after unlock for better responsiveness.
Written for commit 509be03. Summary will update on new commits. Review in cubic