| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
No issues found across 26 files
Confidence score: 5/5
sequenceDiagram
participant Main as Plugin Main
participant Trampoline as NEW: Start Trampoline
participant Mod as eBPF Module
participant Thread as Module Worker Thread
participant Stats as Stats / Shutdown Path
Note over Main, Mod: Thread Startup Flow
Main->>Trampoline: nd_thread_create(ebpf_function_thread_start)
activate Trampoline
Trampoline->>Trampoline: NEW: Park thread (spin on atomic 'ready')
Main->>Main: netdata_mutex_lock(&ebpf_exit_cleanup)
Main->>Mod: NEW: ebpf_module_enabled_set(RUNNING) [Atomic Store]
Main->>Main: Set em->lifetime
Main->>Trampoline: NEW: Set context->ready = true [Atomic Release]
Main->>Main: netdata_mutex_unlock(&ebpf_exit_cleanup)
Trampoline->>Thread: Execute start_routine(em)
deactivate Trampoline
activate Thread
Note over Stats, Mod: Asynchronous Monitoring / Shutdown
Stats->>Mod: NEW: ebpf_module_enabled_get() [Atomic Load]
Note right of Mod: CHANGED: No longer requires global cleanup lock<br/>to check if module is active.
alt Shutdown Signal
Stats->>Thread: nd_thread_signal_cancel()
Thread->>Thread: Perform cleanup
Thread->>Mod: NEW: ebpf_module_enabled_set(STOPPED) [Atomic Store]
deactivate Thread
end
Note over Main, Thread: Functions Event Loop Correction
participant Queue as Functions Job Queue
participant Worker as Event Loop Worker
participant Mutex as Worker Mutex
loop Event Loop
Worker->>Mutex: Lock
Worker->>Queue: NEW: Scan for jobs AND Wait under same lock
alt Job Found
Queue-->>Worker: Return Job
Worker->>Worker: Mark j->running = true
Worker->>Mutex: Unlock
Worker->>Worker: Execute Callback
else No Job
Worker->>Mutex: netdata_cond_wait (atomically release/wait)
Note right of Worker: Prevents missed wakeups between scan and sleep.
end
end
Sorry, something went wrong.
Coverity CID 380968 (MISSING_LOCK): `rrdcalc_unlink_and_delete_all_rrdset_alerts()` read alert values before taking the host alert dictionary write traversal lock, so the health thread could update `rc->value` concurrently. Delete matching alerts under `foreach_rrdcalc_in_rrdhost_write()` to keep the unlink snapshot synchronized with health evaluation.
Coverity CID 439969 (BAD_CHECK_OF_WAIT_COND): the worker loop could miss a new-job signal after failing to acquire runnable work and then sleep even though a request was already queued. Scan for runnable jobs and wait under the same mutex so queued requests cannot be stranded behind a lost wakeup.
|
@cubic-dev-ai write summary in the PR description |
Sorry, something went wrong.
@stelfrag This PR addresses memory-safety and concurrency bugs identified by a Coverity audit, focusing on alert cleanup and worker thread synchronization. Key Changes
Related Resources |
Sorry, something went wrong.
There was a problem hiding this comment.
This PR addresses Coverity-reported correctness and concurrency issues by tightening synchronization around alert cleanup and worker-thread wakeups in the functions event loop.
Changes:
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/libnetdata/functions_evloop/functions_evloop.c | Refactors worker loop locking/cond-wait behavior to avoid missed wakeups when jobs are queued. |
| src/health/rrdcalc.c | Reworks chart-alert cleanup to run under host-level RRDCALC write traversal, avoiding concurrent update races. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
|
Sorry, something went wrong.
There was a problem hiding this comment.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
There was a problem hiding this comment.
Copilot reviewed 2 out of 2 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.
There was a problem hiding this comment.
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
|
@cubic-dev-ai review this PR |
Sorry, something went wrong.
@stelfrag I have started the AI code review. It will take a few minutes to complete. |
Sorry, something went wrong.
There was a problem hiding this comment.
I check different charts from different plugins running this PR, and everything is working as expected. LGTM!
Sorry, something went wrong.
…rt 6) (#22281) * health: serialize rrdset alert cleanup Coverity CID 380968 (MISSING_LOCK): `rrdcalc_unlink_and_delete_all_rrdset_alerts()` read alert values before taking the host alert dictionary write traversal lock, so the health thread could update `rc->value` concurrently. Delete matching alerts under `foreach_rrdcalc_in_rrdhost_write()` to keep the unlink snapshot synchronized with health evaluation. * functions_evloop: fix missed worker wakeup for queued jobs Coverity CID 439969 (BAD_CHECK_OF_WAIT_COND): the worker loop could miss a new-job signal after failing to acquire runnable work and then sleep even though a request was already queued. Scan for runnable jobs and wait under the same mutex so queued requests cannot be stranded behind a lost wakeup. * Address review comments / re-work lock strategy when deleting chart alerts --------- Co-authored-by: Costa Tsaousis <costa@netdata.cloud> (cherry picked from commit 3a902fd)
| Back | FazBrowse Home | New Git URL |
Summary
Summary by cubic
Fixes memory-safety and concurrency bugs found by Coverity by serializing chart alert deletion and preventing lost worker wakeups. Improves correctness during alert cleanup and queued job processing.
Written for commit 5c2685c. Summary will update on new commits. Review in cubic