Bound alert notification execution wait by stelfrag · Pull Request #22626 · netdata/netdata · GitHub

Summary

Fix a hang where one stuck notification process blocks the health thread forever, stalling all alert evaluation and agent shutdown (seen on Windows with wedged msys children).
Add spawn_server_exec_timedwait() / spawn_popen_timedwait() so waits can time out.
Health now kills any notification running past a deadline and keeps going.
New [health] notification execution timeout option (default 2m, 0 = wait forever).
Add spawn-tester coverage and document the new option.

Summary by cubic

Bounds alert notification execution with a configurable timeout so hung notifications can’t stall health checks or shutdown. Adds bounded timed waits with SIGTERM→SIGKILL escalation, a sane default grace, and consistent cross‑platform behavior.

New Features
- Added spawn_server_exec_timedwait() and spawn_popen_timedwait(); waits are bounded, non‑positive timeouts do a minimal poll, and results distinguish RUNNING/EXITED/ERROR (don’t loop on ERROR).
- New health config: notification execution timeout (default 2m, 0 = wait forever), clamped to [0, INT32_MAX].
- Health waits in 1s slices with a monotonic deadline, re-checks shutdown, and kills overdue or errored notifications.
Bug Fixes
- Prevented hangs where stuck notifications blocked health or shutdown (incl. Windows/msys); closed child pipes during waits so I/O‑blocked children can exit.
- Refactored SIGTERM→SIGKILL escalation: use caller grace once, fall back to SPAWN_KILL_DEFAULT_GRACE_MS, and reclaim instances if SIGKILL cannot be confirmed to avoid indefinite waits.
- Treat broken status sockets or process handles as terminal (ERROR) to avoid spinning at 0 timeout; improved error‑path and SIGKILL‑failure logging.
- Made status optional in spawn_server_exec_timedwait() and added null checks across all backends.

Written for commit 1b2f985. Summary will update on new commits.

…with configurable timeouts - Introduced `spawn_server_exec_timedwait` across spawn server implementations (POSIX, libuv, nofork, Windows). - Added timeout-based process cleanup for `POPEN_INSTANCE` via `spawn_popen_timedwait`. - Configurable notification execution timeout (`notification_execution_timeout_seconds`) added to health configuration. - Updated alert notification handling to kill hanging processes after exceeding the timeout. - Extended unit tests to validate timed-wait behavior for processes.

2 issues found across 12 files

Confidence score: 2/5

There is a high-confidence regression risk in src/libnetdata/spawn_server/spawn_server_posix.c: spawn_server_exec_timedwait() may ignore its timeout when interrupted by EINTR, which can lead to indefinite blocking instead of bounded waits.
src/health/health_notifications.c has a similar user-impacting risk: the new timeout/kill path still performs a blocking kill+wait sequence, so timeout-protected flows can still hang forever.
Given two medium-high severity timeout bugs with strong confidence and clear runtime impact, this is not yet safe to merge without follow-up fixes.
Pay close attention to src/libnetdata/spawn_server/spawn_server_posix.c and src/health/health_notifications.c - timeout paths can still block indefinitely.

Architecture diagram

sequenceDiagram
    participant Health as Health Thread
    participant AE as ALARM_ENTRY
    participant SPopen as spawn_popen_timedwait()
    participant SSExec as spawn_server_exec_timedwait()
    participant Child as Notification Process
    participant Config as netdata.conf

    Note over Health,Config: Bound alert notification execution with configurable timeout

    Health->>Config: CHANGED: read "notification execution timeout" (default 2m/120s)
    Config-->>Health: timeout value (0 = wait forever)

    Health->>AE: health_alarm_wait_for_execution(ae)
    AE->>AE: CHANGED: compute deadline = exec_run_timestamp + timeout

    loop every 1 second
        AE->>SPopen: spawn_popen_timedwait(pi, 1000ms, &code)
        SPopen->>SSExec: spawn_server_exec_timedwait(server, si, timeout_ms, &status)
        SSExec->>Child: close pipe FDs, poll for exit

        alt child exited within slice
            Child-->>SSExec: exit status
            SSExec-->>SPopen: SPAWN_TIMEDWAIT_EXITED, status
            SPopen-->>AE: true, code
            AE->>Health: notification completed
            AE->>AE: break loop
        else timeout expired (child still running)
            SSExec-->>SPopen: SPAWN_TIMEDWAIT_RUNNING
            SPopen-->>AE: false, pi still valid
        end
    end

    alt timeout exceeded OR service shutting down
        AE->>AE: CHANGED: check now_realtime_sec() >= deadline OR !service_running(SERVICE_HEALTH)
        AE->>SPopen: spawn_popen_kill(pi, 0)
        SPopen->>Child: force kill process
        Child-->>SPopen: terminated
        SPopen-->>AE: code = 128
        AE->>Health: notification killed, continue evaluation
    end

    AE->>Health: CHANGED: health thread continues evaluation/shutdown immediately

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Pull request overview

This PR prevents the Netdata health thread from hanging indefinitely on stuck alert notification processes by introducing timed-wait APIs in the spawn subsystem and enforcing a configurable notification execution timeout in health notification handling.

Changes:

Added spawn_server_exec_timedwait() and spawn_popen_timedwait() to support bounded waiting for child process termination.
Updated health notification execution to periodically timed-wait and kill notifications that exceed a configured deadline (default 2 minutes, 0 = wait forever).
Added spawn-tester coverage for the timed-wait behavior and documented the new health configuration option.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/libnetdata/spawn_server/spawn-tester.c	Adds tests for timed-wait behavior (child exits vs. requires kill).
src/libnetdata/spawn_server/spawn_server.h	Introduces SPAWN_TIMEDWAIT_RESULT and the spawn_server_exec_timedwait() API.
src/libnetdata/spawn_server/spawn_server_windows.c	Implements timed-wait for Windows via WaitForSingleObject().
src/libnetdata/spawn_server/spawn_server_posix.c	Implements timed-wait for POSIX via waitpid(..., WNOHANG) polling until deadline.
src/libnetdata/spawn_server/spawn_server_nofork.c	Implements timed-wait for nofork mode via socket wait with timeout.
src/libnetdata/spawn_server/spawn_server_libuv.c	Implements timed-wait for libuv mode via semaphore polling until deadline.
src/libnetdata/spawn_server/spawn_popen.h	Adds spawn_popen_timedwait() API documentation and signature.
src/libnetdata/spawn_server/spawn_popen.c	Implements spawn_popen_timedwait() on top of spawn-server timed-wait.
src/health/health.c	Adds and loads the new notification execution timeout config option (with validation).
src/health/health_notifications.c	Enforces bounded notification execution and kills notifications past deadline / during shutdown.
src/health/health_internals.h	Adds notification_execution_timeout_seconds to health globals config.
src/daemon/config/README.md	Documents the new [health] notification execution timeout setting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…g processes - Clamp negative/zero timeouts to positive defaults to avoid infinite waits. - Escalate hanging child processes from SIGTERM to SIGKILL after timeout expiration. - Ensure consistent behavior across spawn server implementations (POSIX, libuv, nofork, Windows). - Refactor health notification timeout handling to avoid indefinite blocking by slow/hanging processes.

@cubic-dev-ai please review again

@cubic-dev-ai please review again

@stelfrag I have started the AI code review. It will take a few minutes to complete.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

…ve timeout behavior - Updated comments to explain bounded timeout behavior (non-infinite waits) for child process management. - Normalized negative timeout values to effectively enable non-blocking polls. - Improved documentation on health notification timeout clamping.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

… processes blocked on I/O.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

4 issues found across 12 files

Confidence score: 3/5

There is some concrete regression risk in src/libnetdata/spawn_server/spawn_server_windows.c: treating WAIT_FAILED as a normal process exit in timed wait can falsely report completion and trigger premature instance cleanup.
src/libnetdata/spawn_server/spawn_server_nofork.c has a race-prone timeout path (SIGKILL without a final liveness check), which could affect the wrong PID if exit/reuse timing lines up.
I’m scoring this a 3 because the top findings are medium severity (6/10) with solid confidence and are in process-lifecycle code where small logic errors can be user-impacting.
Pay close attention to src/libnetdata/spawn_server/spawn_server_windows.c and src/libnetdata/spawn_server/spawn_server_nofork.c - timeout/wait semantics may misreport child state or kill incorrectly near exit boundaries.

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

…ement - Updated comments to explain PID reuse prevention and bounded timeout behavior. - Enhanced documentation for timeout clamping and child process escalation (SIGKILL after SIGTERM). - Improved error handling for Windows process waits (reporting failed handles as running instead of exited).

- Updated `SPAWN_TIMEDWAIT_ERROR` handling to distinguish terminal errors from transient "still running" states. - Prevent looping on broken channels or handles to avoid infinite spins. - Refactored escalation logic for SIGKILL if process wait cannot be completed.

@cubic-dev-ai please review again

@cubic-dev-ai please review again

@stelfrag I have started the AI code review. It will take a few minutes to complete.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 9 comments.

1 issue found across 12 files

Confidence score: 4/5

This looks safe to merge with minimal risk: the reported problem is confined to test logic in src/libnetdata/spawn_server/spawn-tester.c, not an obvious production-path break.
The main issue is that spawn_popen_timedwait() ERROR handling is incorrect in the new timedwait test loop, which could misreport behavior or hide real failures in CI.
Given the 6/10 severity and high confidence (9/10), this is worth fixing soon, but it appears more like a test reliability issue than a direct runtime regression.
Pay close attention to src/libnetdata/spawn_server/spawn-tester.c - fix the timedwait loop to handle ERROR correctly and avoid invalid retry behavior.

Architecture diagram

sequenceDiagram
    participant HealthThread as Health Thread
    participant NotificationWait as health_alarm_wait_for_execution()
    participant SpawnPopen as spawn_popen_timedwait()
    participant SpawnServer as spawn_server_exec_timedwait()
    participant ChildProcess as Notification Script
    participant Config as Health Config

    Note over HealthThread,Config: NEW: Configurable notification execution timeout (default 2m)

    Config->>HealthThread: Load notification_execution_timeout_seconds (clamped)
    HealthThread->>HealthThread: Run notification script (via spawn_popen_run)

    HealthThread->>NotificationWait: Wait for notification to finish

    NotificationWait->>NotificationWait: Get timeout from health_globals.config
    NotificationWait->>NotificationWait: Compute monotonic deadline

    loop Every 1s slice
        NotificationWait->>SpawnPopen: spawn_popen_timedwait(pi, 1000, &code)
        SpawnPopen->>SpawnPopen: Close child pipes
        SpawnPopen->>SpawnServer: spawn_server_exec_timedwait(..., 1000, &status)
        SpawnServer->>SpawnServer: Wait with timeout (backend-specific)
        alt Child exited normally
            SpawnServer-->>SpawnPopen: SPAWN_TIMEDWAIT_EXITED (status)
            SpawnPopen->>SpawnPopen: Free instance, set code
            SpawnPopen-->>NotificationWait: SPAWN_TIMEDWAIT_EXITED
            NotificationWait->>NotificationWait: Break loop
        else Child still running
            SpawnServer-->>SpawnPopen: SPAWN_TIMEDWAIT_RUNNING
            SpawnPopen-->>NotificationWait: SPAWN_TIMEDWAIT_RUNNING
            NotificationWait->>NotificationWait: Check deadline and service_running()
            alt Deadline reached or shutdown signalled
                NotificationWait->>SpawnPopen: spawn_popen_kill(pi, 0)
                SpawnPopen->>SpawnServer: spawn_server_exec_kill (SIGTERM→SIGKILL escalation)
                SpawnServer->>ChildProcess: Send SIGTERM, then SIGKILL
                SpawnServer-->>SpawnPopen: Wait completed (status)
                SpawnPopen-->>NotificationWait: code = 128
                NotificationWait->>NotificationWait: Break loop, log kill
            else Still within deadline
                NotificationWait->>NotificationWait: Continue loop (sleep until next slice)
            end
        else Status channel error (ERROR)
            SpawnServer-->>SpawnPopen: SPAWN_TIMEDWAIT_ERROR
            SpawnPopen-->>NotificationWait: SPAWN_TIMEDWAIT_ERROR
            NotificationWait->>SpawnPopen: spawn_popen_kill (immediate)
            Note over NotificationWait: ERROR is terminal – do not loop
        end
    end

    NotificationWait-->>HealthThread: Notification finished (code)
    HealthThread->>HealthThread: Continue health evaluation loop

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

…oss all implementations - Add null checks for `status` before assignment to enhance flexibility and prevent potential null pointer dereference. - Update logic in POSIX, libuv, nofork, and Windows implementations. - Improve documentation to clarify `status` as an optional parameter.

@cubic-dev-ai please review again

@cubic-dev-ai please review again

@stelfrag I have started the AI code review. It will take a few minutes to complete.

2 issues found across 12 files

Confidence score: 3/5

There is moderate merge risk because spawn_server_exec_kill() in src/libnetdata/spawn_server/spawn_server_libuv.c ignores its timeout_ms argument and always uses 2000ms, which can cause caller-configured shutdown timing to be silently wrong.
src/libnetdata/spawn_server/spawn-tester.c exits immediately on unexpected timedwait ERROR paths, and that can leak a still-owned child instance; this is less severe than the timeout bug but still a concrete cleanup/regression concern.
Given one medium-severity, high-confidence behavioral issue in runtime code and another high-confidence robustness issue, this is not a merge-blocker but does carry user-impacting risk.
Pay close attention to src/libnetdata/spawn_server/spawn_server_libuv.c, src/libnetdata/spawn_server/spawn-tester.c - timeout control is being bypassed in one path and child ownership cleanup can be skipped in error handling.

Architecture diagram

sequenceDiagram
    participant Health as Health Thread
    participant Notif as Notification Queue
    participant Popen as popen (libnetdata)
    participant SpawnSrv as Spawn Server
    participant ChildProc as Child Process (notification script)
    participant Config as Config Store

    Note over Health,Config: NEW: Bounded notification execution flow

    Health->>Config: Read notification_execution_timeout_seconds
    Config-->>Health: value (default 2m, 0=forever, clamped [0, INT32_MAX])

    Health->>Notif: Pop next ALARM_ENTRY to wait for
    Notif-->>Health: ae with popen_instance

    loop Every 1s slice until child exits, shutdown, or deadline
        Health->>Popen: spawn_popen_timedwait(pi, 1000ms, &code)
        Popen->>Popen: Close child pipes (force EOF/exit)
        Popen->>SpawnSrv: spawn_server_exec_timedwait(server, si, timeout_ms, &status)

        alt Child Exited Normally
            SpawnSrv->>SpawnSrv: Wait for process exit
            SpawnSrv-->>Popen: SPAWN_TIMEDWAIT_EXITED + status
            Popen->>Popen: freez(pi)
            Popen-->>Health: SPAWN_TIMEDWAIT_EXITED, code
            Health->>Health: Break loop, record exec_code

        else Child Still Running
            SpawnSrv-->>Popen: SPAWN_TIMEDWAIT_RUNNING
            Popen-->>Health: SPAWN_TIMEDWAIT_RUNNING
            Health->>Health: Check deadline_ut vs now_monotonic_usec()
            Health->>Health: Check service_running(SERVICE_HEALTH)
            opt Deadline Reached OR Shutdown Signal
                Health->>Popen: spawn_popen_kill(pi, 0)
                Popen->>Popen: Close pipes
                Popen->>SpawnSrv: spawn_server_exec_kill(server, si, 0)
                alt SIGTERM handled
                    SpawnSrv->>ChildProc: SIGTERM
                    SpawnSrv->>SpawnSrv: spawn_server_exec_timedwait(..., 2000ms)
                    alt Still alive after 2s
                        SpawnSrv->>ChildProc: SIGKILL (or equivalent)
                    end
                else SIGTERM ignored or status channel broken
                    SpawnSrv->>ChildProc: SIGKILL (or equivalent)
                end
                SpawnSrv-->>Popen: status
                Popen-->>Health: code = 128
                Health->>Health: Break loop, record exec_code = 128
            end

        else Wait Error (broken status channel / handle)
            SpawnSrv-->>Popen: SPAWN_TIMEDWAIT_ERROR
            Popen-->>Health: SPAWN_TIMEDWAIT_ERROR
            Health->>Health: Log error, treat as terminal
            Health->>Popen: spawn_popen_kill(pi, 0) to reclaim
            Health->>Health: Break loop, record exec_code = 128
        end
    end

    Health->>Health: Set ae->popen_instance = NULL
    Health->>Health: Store exec_code in ALARM_ENTRY

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

…s spawn server implementations - Added `SPAWN_KILL_DEFAULT_GRACE_MS` for consistent default grace periods on negative/zero timeouts. - Updated timeout calculations to ensure bounded termination waits before escalating from SIGTERM to SIGKILL. - Refactored logic in POSIX, libuv, and nofork implementations for clarity and consistency. - Improved error handling and cleanup in `spawn_popen_timedwait` to address terminal error scenarios.

@cubic-dev-ai please review again

@cubic-dev-ai please review again

@stelfrag I have started the AI code review. It will take a few minutes to complete.

2 issues found across 13 files

Confidence score: 3/5

There is a concrete regression risk in src/libnetdata/spawn_server/spawn_server_posix.c: SIGKILL escalation ignores kill() failure and then performs a blocking wait, which can still hang process shutdown paths.
src/libnetdata/spawn_server/spawn_server_nofork.c appears to apply the timeout twice in spawn_server_exec_kill(), which can delay escalation beyond the configured grace period and make termination behavior inconsistent.
Given two medium-to-high severity process-control issues (6–7/10) with plausible runtime impact, this sits in moderate merge risk rather than a safe-merge state.
Pay close attention to src/libnetdata/spawn_server/spawn_server_posix.c, src/libnetdata/spawn_server/spawn_server_nofork.c - kill escalation and timeout handling may hang or delay process termination.

Architecture diagram

sequenceDiagram
    participant Health as Health Plugin
    participant Config as Health Config
    participant Notif as Notification System
    participant Spawn as Spawn Server
    participant Child as Notification Child Process
    participant Scheduler as Agent Scheduler

    Note over Health,Child: CHANGED: Bounded notification execution

    Health->>Config: Read notification_execution_timeout_seconds
    Config-->>Health: 120s (default) or user-configured value

    Health->>Notif: spawn_popen_run(alarm-notify.sh)
    Notif->>Spawn: spawn_server_exec()
    Spawn->>Child: fork/exec notification command
    Child-->>Spawn: Running
    Spawn-->>Notif: POPEN_INSTANCE
    Notif-->>Health: ae with popen_instance

    Note over Health,Spawn: NEW: Timed wait loop (1s slices)

    loop Every HEALTH_NOTIFICATION_WAIT_SLICE_MS (1000ms)
        Health->>Notif: spawn_popen_timedwait(pi, 1000ms, &code)
        Notif->>Spawn: spawn_server_exec_timedwait(timeout_ms)
        
        alt Child has exited
            spawn_server_exec_timedwait ->> Child: Wait completed (EXITED)
            Spawn-->>Notif: SPAWN_TIMEDWAIT_EXITED
            Notif->>Notif: freez(pi), set code
            Notif-->>Health: break loop
        else Timeout expired, child still running
            Spawn-->>Notif: SPAWN_TIMEDWAIT_RUNNING
            Notif-->>Health: continue loop
        else Status channel error (broken socket/handle)
            Spawn-->>Notif: SPAWN_TIMEDWAIT_ERROR
            Notif-->>Health: continue to kill logic
        end

        Health->>Health: Check if service_running(SERVICE_HEALTH)
        Health->>Health: Check if deadline_reached (timeout > 0)
        
        alt Shutdown requested or deadline reached or ERROR
            Health->>Notif: spawn_popen_kill(pi, 0)
            Notif->>Spawn: spawn_server_exec_kill()
            Spawn->>Child: SIGTERM (or TerminateProcess on Windows)
            
            alt Child ignores SIGTERM after grace period
                Spawn->>Child: SIGKILL escalation
            end
            
            Spawn-->>Notif: Child exited
            Notif-->>Health: code = 128
            Health->>Health: break loop
        end
    end

    Note over Health,Scheduler: NEW: Re-check shutdown on every slice
    
    alt Shutdown in progress
        Scheduler->>Health: service_running(SERVICE_HEALTH) = false
        Health->>Health: Terminate notification wait early
    end

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

…ing in spawn server implementations - Added logging for SIGKILL failures in nofork, libuv, and POSIX implementations to enhance debugging. - Clarified timeout behavior to prevent applying caller-defined grace periods twice. - Updated comments to explain SIGKILL escalation scenarios and terminal process states.

1 issue found across 3 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

… cleanup and prevent indefinite waits

You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment @cubic-dev-ai review.

@cubic-dev-ai please review again

@cubic-dev-ai please review again

@stelfrag I have started the AI code review. It will take a few minutes to complete.

1 issue found across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

When using a fake script, I still had alerts raised. LGTM!

* Add `spawn_server_exec_timedwait` to prevent hanging child processes with configurable timeouts - Introduced `spawn_server_exec_timedwait` across spawn server implementations (POSIX, libuv, nofork, Windows). - Added timeout-based process cleanup for `POPEN_INSTANCE` via `spawn_popen_timedwait`. - Configurable notification execution timeout (`notification_execution_timeout_seconds`) added to health configuration. - Updated alert notification handling to kill hanging processes after exceeding the timeout. - Extended unit tests to validate timed-wait behavior for processes. * Add process timeout clamping and SIGKILL escalation to prevent hanging processes - Clamp negative/zero timeouts to positive defaults to avoid infinite waits. - Escalate hanging child processes from SIGTERM to SIGKILL after timeout expiration. - Ensure consistent behavior across spawn server implementations (POSIX, libuv, nofork, Windows). - Refactor health notification timeout handling to avoid indefinite blocking by slow/hanging processes. * Clarify timeout handling for child process waits and normalize negative timeout behavior - Updated comments to explain bounded timeout behavior (non-infinite waits) for child process management. - Normalized negative timeout values to effectively enable non-blocking polls. - Improved documentation on health notification timeout clamping. * Close child pipes in `spawn_server_exec_timedwait` to prevent hanging processes blocked on I/O. * Clarify timeout handling and escalation logic for child process management - Updated comments to explain PID reuse prevention and bounded timeout behavior. - Enhanced documentation for timeout clamping and child process escalation (SIGKILL after SIGTERM). - Improved error handling for Windows process waits (reporting failed handles as running instead of exited). * Improve error handling in spawn server and popen implementations - Handle status socket errors during `spawn_server_exec_wait` to avoid premature child cleanup and escalate to kill if needed. - Fix null pointer dereference issues in `spawn_popen_timedwait` by adding safety checks before assigning codes. * Resolve wait handling for terminal errors in `spawn_server_exec_timedwait` - Updated logic to treat status socket and process handle errors as terminal instead of transient "still running" states. - Prevent infinite spinning on broken channels or unusable handles during timed waits. - Improved logging for error resolution paths on both POSIX and Windows platforms. * Handle terminal errors in process wait and prevent infinite loops - Updated `SPAWN_TIMEDWAIT_ERROR` handling to distinguish terminal errors from transient "still running" states. - Prevent looping on broken channels or handles to avoid infinite spins. - Refactored escalation logic for SIGKILL if process wait cannot be completed. * Make `status` parameter optional in `spawn_server_exec_timedwait` across all implementations - Add null checks for `status` before assignment to enhance flexibility and prevent potential null pointer dereference. - Update logic in POSIX, libuv, nofork, and Windows implementations. - Improve documentation to clarify `status` as an optional parameter. * Normalize timeout handling and enhance SIGKILL escalation logic across spawn server implementations - Added `SPAWN_KILL_DEFAULT_GRACE_MS` for consistent default grace periods on negative/zero timeouts. - Updated timeout calculations to ensure bounded termination waits before escalating from SIGTERM to SIGKILL. - Refactored logic in POSIX, libuv, and nofork implementations for clarity and consistency. - Improved error handling and cleanup in `spawn_popen_timedwait` to address terminal error scenarios. * Improve error handling for SIGKILL failures and clarify timeout handling in spawn server implementations - Added logging for SIGKILL failures in nofork, libuv, and POSIX implementations to enhance debugging. - Clarified timeout behavior to prevent applying caller-defined grace periods twice. - Updated comments to explain SIGKILL escalation scenarios and terminal process states. * Refactor SIGTERM-to-SIGKILL escalation logic to improve child process cleanup and prevent indefinite waits (cherry picked from commit 4381b68)

github-actions Bot added area/docs area/health area/daemon labels Jun 4, 2026

cubic-dev-ai Bot reviewed Jun 4, 2026

View reviewed changes

Comment thread src/libnetdata/spawn_server/spawn_server_posix.c Outdated

Comment thread src/health/health_notifications.c

stelfrag marked this pull request as ready for review June 4, 2026 06:55

stelfrag requested a review from Ancairon as a code owner June 4, 2026 06:55

Copilot AI review requested due to automatic review settings June 4, 2026 06:55

stelfrag requested review from kanelatechnical, thiagoftsm and vkalintiris as code owners June 4, 2026 06:55

stelfrag marked this pull request as draft June 4, 2026 06:55

Copilot started reviewing on behalf of stelfrag June 4, 2026 06:55 View session

Copilot AI reviewed Jun 4, 2026

View reviewed changes

stelfrag requested a review from Copilot June 4, 2026 12:47

Copilot started reviewing on behalf of stelfrag June 4, 2026 12:47 View session

Copilot AI reviewed Jun 4, 2026

View reviewed changes

Comment thread src/health/health.c Outdated

Comment thread src/libnetdata/spawn_server/spawn_server.h Outdated

Comment thread src/libnetdata/spawn_server/spawn_popen.h Outdated

stelfrag requested a review from Copilot June 4, 2026 12:54

Copilot started reviewing on behalf of stelfrag June 4, 2026 12:54 View session

Copilot AI reviewed Jun 4, 2026

View reviewed changes

Comment thread src/libnetdata/spawn_server/spawn_server_posix.c

Close child pipes in spawn_server_exec_timedwait to prevent hanging…

11aed1a

… processes blocked on I/O.

stelfrag requested a review from Copilot June 4, 2026 13:01

Copilot started reviewing on behalf of stelfrag June 4, 2026 13:02 View session

Copilot AI reviewed Jun 4, 2026

View reviewed changes

Comment thread src/health/health_notifications.c Outdated

cubic-dev-ai Bot reviewed Jun 4, 2026

View reviewed changes

Comment thread src/libnetdata/spawn_server/spawn_server_windows.c Outdated

Comment thread src/libnetdata/spawn_server/spawn_server_nofork.c

Comment thread src/libnetdata/spawn_server/spawn_server_nofork.c Outdated

Comment thread src/health/health_notifications.c Outdated

stelfrag requested a review from Copilot June 5, 2026 07:37

Copilot started reviewing on behalf of stelfrag June 5, 2026 07:37 View session

Copilot AI reviewed Jun 5, 2026

View reviewed changes

cubic-dev-ai Bot reviewed Jun 5, 2026

View reviewed changes

Comment thread src/libnetdata/spawn_server/spawn-tester.c Outdated

cubic-dev-ai Bot reviewed Jun 5, 2026

View reviewed changes

Comment thread src/libnetdata/spawn_server/spawn_server_libuv.c Outdated

Comment thread src/libnetdata/spawn_server/spawn-tester.c Outdated

cubic-dev-ai Bot reviewed Jun 8, 2026

View reviewed changes

Comment thread src/libnetdata/spawn_server/spawn_server_posix.c Outdated

Comment thread src/libnetdata/spawn_server/spawn_server_nofork.c Outdated

stelfrag marked this pull request as ready for review June 8, 2026 07:45

cubic-dev-ai Bot reviewed Jun 8, 2026

View reviewed changes

Comment thread src/libnetdata/spawn_server/spawn_server_nofork.c Outdated

Refactor SIGTERM-to-SIGKILL escalation logic to improve child process…

1b2f985

… cleanup and prevent indefinite waits

cubic-dev-ai Bot reviewed Jun 8, 2026

View reviewed changes

Comment thread src/libnetdata/spawn_server/spawn_server_nofork.c

thiagoftsm approved these changes Jun 8, 2026

View reviewed changes

stelfrag merged commit 4381b68 into netdata:master Jun 9, 2026
200 of 201 checks passed

stelfrag deleted the health-notification-wait-timeout branch June 9, 2026 06:55

stelfrag mentioned this pull request Jun 22, 2026

Patch release 2.10.4 #22825

Merged

github-actions Bot mentioned this pull request Aug 3, 2026

Update dependency netdata/netdata to v2.10.4 claytono/infra#2667

Merged

1 task

Bound alert notification execution wait by stelfrag · Pull Request #22626 · netdata/netdata · GitHub

Uh oh!

Bound alert notification execution wait - #22626

Bound alert notification execution wait#22626stelfrag merged 12 commits intonetdata:masternetdata/netdata:masterfrom stelfrag:health-notification-wait-timeoutstelfrag/netdata:health-notification-wait-timeoutCopy head branch name to clipboard

Conversation

stelfrag commented Jun 4, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Summary by cubic

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stelfrag commented Jun 4, 2026

Uh oh!

cubic-dev-ai Bot commented Jun 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stelfrag commented Jun 5, 2026

Uh oh!

cubic-dev-ai Bot commented Jun 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stelfrag commented Jun 5, 2026

Uh oh!

cubic-dev-ai Bot commented Jun 5, 2026

Bound alert notification execution wait#22626
stelfrag merged 12 commits into
netdata:masterfrom
stelfrag:health-notification-wait-timeout

stelfrag commented Jun 4, 2026 •
edited by cubic-dev-ai Bot
Loading

cubic-dev-ai Bot left a comment •
edited
Loading

cubic-dev-ai Bot left a comment •
edited
Loading

cubic-dev-ai Bot left a comment •
edited
Loading

cubic-dev-ai Bot left a comment •
edited
Loading

cubic-dev-ai Bot left a comment •
edited
Loading

cubic-dev-ai Bot left a comment •
edited
Loading

cubic-dev-ai Bot left a comment •
edited
Loading