@cryptotaxi247 / netdata / commits / 4381b68aa

Bound alert notification execution wait (#22626)

* 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

Stelios Fragkakis committed Jun 9, 2026 at 09:55 UTC 4381b68aa843821edb76726be10ce521f082a555

13 files changed +398 -5

src/daemon/config/README.md

@@ -217,6 +217,7 @@ Specific Alerts are configured in per-collector config files under the `health.d

217 | in memory max Health log entries | 1000 | Size of the Alert history held in RAM |

218 | script to execute on alarm | `/usr/libexec/netdata/plugins.d/alarm-notify.sh` | The script that sends Alert notifications. Note that in versions before 1.16, the plugins.d directory may be installed in a different location in certain OSs (e.g. under `/usr/lib/netdata`). |

219 | run at least every | `10s` | Controls how often all Alert conditions should be evaluated. |

220 +| notification execution timeout | `2m` | How long a notification command (e.g. `alarm-notify.sh`) may run before it is killed. Protects Alert evaluation from hung notification processes. Set to `0` to wait forever. |

221 | postpone alarms during hibernation for | `1m` | Prevents false Alerts. May need to be increased if you get Alerts during hibernation. |

222 | Health log retention | `5d` | Specifies the history of Alert events (in seconds) kept in the Agent's sqlite database. |

223 | enabled alarms | * | Defines which Alerts to load from both user and stock directories. This is a [simple pattern](/src/libnetdata/simple_pattern/README.md) list of Alert or template names. Can be used to disable specific Alerts. For example, `enabled alarms = !oom_kill *` will load all Alerts except `oom_kill`. |

src/health/health.c

+12

@@ -21,6 +21,7 @@ struct health_plugin_globals health_globals = {

22 .run_at_least_every_seconds = 10,

23 .postpone_alarms_during_hibernation_for_seconds = 60,

24 + .notification_execution_timeout_seconds = 120,

25 },

26 .prototypes = {

27 .dict = NULL,

@@ -89,6 +90,17 @@ void health_load_config_defaults(void) {

90 "postpone alarms during hibernation for",

91 health_globals.config.postpone_alarms_during_hibernation_for_seconds);

93 + time_t notification_execution_timeout =

94 + inicfg_get_duration_seconds(&netdata_config, CONFIG_SECTION_HEALTH,

95 + "notification execution timeout",

96 + health_globals.config.notification_execution_timeout_seconds);

97 + // clamp to [0, INT32_MAX] before narrowing to int32: the upper clamp prevents a huge

98 + // value from overflowing into a negative, the lower clamp normalizes negatives to 0.

99 + // 0 means "wait forever".

100 + if(notification_execution_timeout < 0) notification_execution_timeout = 0;

101 + if(notification_execution_timeout > INT32_MAX) notification_execution_timeout = INT32_MAX;

102 + health_globals.config.notification_execution_timeout_seconds = (int32_t)notification_execution_timeout;

103 +

104 health_globals.config.default_recipient =

105 string_strdupz("root");

106

src/health/health_internals.h

@@ -88,6 +88,7 @@ struct health_plugin_globals {

89 int32_t run_at_least_every_seconds;

90 int32_t postpone_alarms_during_hibernation_for_seconds;

91 + int32_t notification_execution_timeout_seconds; // kill notification commands running longer than this (0 = wait forever)

92 } config;

94 struct {

src/health/health_notifications.c

+33 -2

@@ -6,6 +6,9 @@

6 // the queue of executed alarm notifications that haven't been waited for yet

7 static ALARM_ENTRY *alarm_notifications_in_progress = NULL;

9 +// how often the notification wait loop wakes up to re-check shutdown and the deadline

10 +#define HEALTH_NOTIFICATION_WAIT_SLICE_MS 1000

11 +

12 struct health_raised_summary {

13 RRDHOST *host;

14 DICTIONARY *rrdcalc_dict;

@@ -35,9 +38,37 @@ void health_alarm_wait_for_execution(ALARM_ENTRY *ae) {

38 goto cleanup;

39 }

38 - code = spawn_popen_wait(ae->popen_instance);

41 + // bound the wait so a hung notification process (seen on Windows, where msys children can

42 + // wedge during startup) cannot block the single health thread - and with it all health

43 + // evaluation. Each slice is always bounded; the overall wait is bounded only when a non-zero

44 + // timeout is configured. timeout == 0 means "wait forever" - the loop then breaks only on

45 + // child exit or shutdown. The deadline is monotonic, so a wall-clock jump cannot extend it.

46 + int32_t timeout = health_globals.config.notification_execution_timeout_seconds;

47 + usec_t deadline_ut = now_monotonic_usec() + (usec_t)timeout * USEC_PER_SEC;

48 +

49 + while(true) {

50 + SPAWN_TIMEDWAIT_RESULT r = spawn_popen_timedwait(ae->popen_instance, HEALTH_NOTIFICATION_WAIT_SLICE_MS, &code);

51 + if(r == SPAWN_TIMEDWAIT_EXITED)

52 + break;

53 +

54 + // RUNNING: keep waiting unless we should stop. ERROR: the wait broke and must never be

55 + // looped on (it would spin forever at timeout == 0), so always fall through to the kill.

56 + // re-check shutdown every slice, so a slow notification cannot block agent exit.

57 + bool deadline_reached = (timeout > 0 && now_monotonic_usec() >= deadline_ut);

58 + if(r == SPAWN_TIMEDWAIT_ERROR || unlikely(!service_running(SERVICE_HEALTH)) || deadline_reached) {

59 + nd_log(NDLS_DAEMON, NDLP_ERR,

60 + "HEALTH: alert notification '%s' (pid %d) %s - killing it",

61 + ae_name(ae), (int)spawn_popen_pid(ae->popen_instance),

62 + (r == SPAWN_TIMEDWAIT_ERROR) ? "could not be waited for (status channel error)"

63 + : "is still running past its execution timeout");

64 +

65 + spawn_popen_kill(ae->popen_instance, 0);

66 + code = 128;

67 + break;

68 + }

69 + }

70 ae->popen_instance = NULL;

40 - netdata_log_debug(D_HEALTH, "done executing command - returned with code %d", ae->exec_code);

71 + netdata_log_debug(D_HEALTH, "done executing command - returned with code %d", code);

73 cleanup:

74 ae->exec_code = code;

src/libnetdata/spawn_server/spawn-tester.c

+100

@@ -389,12 +389,104 @@ void test_popen_plugin_echo_and_exit(const char *argv0) {

389 }

390 }

391

392 +// --------------------------------------------------------------------------------------------------------------------

393 +// timed wait

394 +

395 +int plugin_sleep_to_stop(void) {

396 + child_check_fds();

397 + child_check_environment();

398 +

399 + // ignore the pipes - only a kill can stop us within the test's lifetime

400 + sleep_usec(3600 * USEC_PER_SEC);

401 + return 0;

402 +}

403 +

404 +void test_popen_plugin_timedwait_exits(const char *argv0) {

405 + // a child that exits on its own must be reaped by spawn_popen_timedwait()

406 + char cmd[FILENAME_MAX + 100];

407 + snprintfz(cmd, sizeof(cmd), "exec %s plugin-echo-and-exit", argv0);

408 + POPEN_INSTANCE *pi = spawn_popen_run(cmd);

409 + if(!pi) {

410 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot run myself as plugin (popen)");

411 + exit(1);

412 + }

413 +

414 + int code = -1;

415 + size_t slices = 0;

416 + for(;;) {

417 + SPAWN_TIMEDWAIT_RESULT r = spawn_popen_timedwait(pi, 100, &code);

418 + if(r == SPAWN_TIMEDWAIT_EXITED)

419 + break;

420 +

421 + if(r == SPAWN_TIMEDWAIT_ERROR) {

422 + // ERROR must never be looped over; for a cleanly-exiting child it should not happen at all.

423 + // pi is still owned on ERROR, so reclaim it before bailing out.

424 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "spawn_popen_timedwait() returned ERROR for a child that should exit cleanly");

425 + spawn_popen_kill(pi, 0);

426 + exit(1);

427 + }

428 +

429 + // SPAWN_TIMEDWAIT_RUNNING - pi is still owned

430 + if(++slices > 100) {

431 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "spawn_popen_timedwait() did not reap a child that exits immediately");

432 + spawn_popen_kill(pi, 0);

433 + exit(1);

434 + }

435 + }

436 +

437 + nd_log(NDLS_COLLECTORS, NDLP_ERR,

438 + "child exited with code %d (after %zu timedwait slices)",

439 + code, slices);

440 +

441 + if(code != 0) {

442 + nd_log(NDLS_COLLECTORS, NDLP_WARNING, "child should exit with code 0, but exited with code %d", code);

443 + warnings++;

444 + }

445 +}

446 +

447 +void test_popen_plugin_timedwait_kill(const char *argv0) {

448 + // a child that never exits must be reported still-running on every slice, then killed

449 + char cmd[FILENAME_MAX + 100];

450 + snprintfz(cmd, sizeof(cmd), "exec %s plugin-sleep-to-stop", argv0);

451 + POPEN_INSTANCE *pi = spawn_popen_run(cmd);

452 + if(!pi) {

453 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot run myself as plugin (popen)");

454 + exit(1);

455 + }

456 +

457 + int code = 0;

458 + for(size_t i = 0; i < 5; i++) {

459 + SPAWN_TIMEDWAIT_RESULT r = spawn_popen_timedwait(pi, 200, &code);

460 + if(r != SPAWN_TIMEDWAIT_RUNNING) {

461 + nd_log(NDLS_COLLECTORS, NDLP_ERR,

462 + "spawn_popen_timedwait() did not report RUNNING for a sleeping child");

463 + // on ERROR we still own pi and must reclaim it; on EXITED it was already freed

464 + if(r == SPAWN_TIMEDWAIT_ERROR) spawn_popen_kill(pi, 0);

465 + exit(1);

466 + }

467 + }

468 +

469 + code = spawn_popen_kill(pi, 0);

470 +

471 + nd_log(NDLS_COLLECTORS, NDLP_ERR,

472 + "child killed, exited with code %d",

473 + code);

474 +

475 + if(code != 0) {

476 + nd_log(NDLS_COLLECTORS, NDLP_WARNING, "killed child should report code 0, but reported code %d", code);

477 + warnings++;

478 + }

479 +}

480 +

481 // --------------------------------------------------------------------------------------------------------------------

482

483 int main(int argc, const char **argv) {

484 if(argc > 1 && strcmp(argv[1], "plugin-kill-to-stop") == 0)

485 return plugin_kill_to_stop();

486

487 + if(argc > 1 && strcmp(argv[1], "plugin-sleep-to-stop") == 0)

488 + return plugin_sleep_to_stop();

489 +

490 if(argc > 1 && strcmp(argv[1], "plugin-echo-and-exit") == 0)

491 return plugin_echo_and_exit();

492

@@ -442,6 +534,14 @@ int main(int argc, const char **argv) {

534 fprintf(stderr, "\n\nTESTING popen No %zu (close to stop)\n\n", i + 1);

535 test_popen_plugin_close_to_stop(argv[0]);

536 }

537 + for(size_t i = 0; i < 5; i++) {

538 + fprintf(stderr, "\n\nTESTING popen No %zu (timedwait exits)\n\n", i + 1);

539 + test_popen_plugin_timedwait_exits(argv[0]);

540 + }

541 + for(size_t i = 0; i < 5; i++) {

542 + fprintf(stderr, "\n\nTESTING popen No %zu (timedwait kill)\n\n", i + 1);

543 + test_popen_plugin_timedwait_kill(argv[0]);

544 + }

545 netdata_main_spawn_server_cleanup();

546

547 fprintf(stderr, "\n\nTests passed! (%zu warnings)\n\n", warnings);

src/libnetdata/spawn_server/spawn_popen.c

+20

@@ -183,6 +183,26 @@ int spawn_popen_wait(POPEN_INSTANCE *pi) {

183 return spawn_popen_status_rc(status);

184 }

185

186 +SPAWN_TIMEDWAIT_RESULT spawn_popen_timedwait(POPEN_INSTANCE *pi, int timeout_ms, int *code) {

187 + if(!pi) {

188 + if(code) *code = -1;

189 + return SPAWN_TIMEDWAIT_EXITED;

190 + }

191 +

192 + spawn_popen_close_files(pi);

193 +

194 + int status = 0;

195 + SPAWN_TIMEDWAIT_RESULT rc = spawn_server_exec_timedwait(netdata_main_spawn_server, pi->si, timeout_ms, &status);

196 + if(rc != SPAWN_TIMEDWAIT_EXITED)

197 + // RUNNING or ERROR: pi->si is still valid, so pi stays alive for the caller

198 + return rc;

199 +

200 + // EXITED: spawn_server_exec_timedwait() has freed pi->si; free the wrapper too

201 + freez(pi);

202 + if(code) *code = spawn_popen_status_rc(status);

203 + return SPAWN_TIMEDWAIT_EXITED;

204 +}

205 +

206 int spawn_popen_kill(POPEN_INSTANCE *pi, int timeout_ms) {

207 if(!pi) return -1;

208

src/libnetdata/spawn_server/spawn_popen.h

+11

@@ -15,6 +15,17 @@ POPEN_INSTANCE *spawn_popen_run(const char *cmd);

15 POPEN_INSTANCE *spawn_popen_run_argv(const char **argv);

16 POPEN_INSTANCE *spawn_popen_run_variadic(const char *cmd, ...);

17 int spawn_popen_wait(POPEN_INSTANCE *pi);

18 +

19 +// Wait for the child for up to timeout_ms. A non-positive timeout_ms performs a single, minimal

20 +// bounded poll; the wait is always bounded, never infinite.

21 +// SPAWN_TIMEDWAIT_EXITED: the child exited; *code holds its spawn_popen_wait()-style return code

22 +// and pi has been freed.

23 +// SPAWN_TIMEDWAIT_RUNNING: still running after timeout_ms; pi remains valid - call again, or

24 +// spawn_popen_wait()/spawn_popen_kill().

25 +// SPAWN_TIMEDWAIT_ERROR: the wait could not be completed (the child's state is unknown); pi remains

26 +// valid and the caller must reclaim it with spawn_popen_kill(). Do NOT loop on ERROR.

27 +SPAWN_TIMEDWAIT_RESULT spawn_popen_timedwait(POPEN_INSTANCE *pi, int timeout_ms, int *code);

28 +

29 int spawn_popen_kill(POPEN_INSTANCE *pi, int timeout_ms);

31 pid_t spawn_popen_pid(POPEN_INSTANCE *pi);

src/libnetdata/spawn_server/spawn_server.h

+20

@@ -47,6 +47,26 @@ SPAWN_INSTANCE* spawn_server_exec(SPAWN_SERVER *server, int stderr_fd, int custo

47 int spawn_server_exec_kill(SPAWN_SERVER *server, SPAWN_INSTANCE *si, int timeout_ms);

48 int spawn_server_exec_wait(SPAWN_SERVER *server, SPAWN_INSTANCE *si);

50 +typedef enum __attribute__((packed)) {

51 + SPAWN_TIMEDWAIT_EXITED = 0, // the child exited; *status holds its status and si has been freed

52 + SPAWN_TIMEDWAIT_RUNNING = 1, // the timeout expired; the child is still running and si remains valid

53 + SPAWN_TIMEDWAIT_ERROR = 2, // the wait could not be completed (broken status channel / unusable

54 + // handle); the child's state is unknown, si remains valid, and the

55 + // caller must reclaim it (e.g. via spawn_server_exec_kill())

56 +} SPAWN_TIMEDWAIT_RESULT;

57 +

58 +// Wait for the child for up to timeout_ms. A non-positive timeout_ms performs a single, minimal

59 +// bounded poll (the nofork backend uses a ~1ms slice because its wait primitive treats 0 as

60 +// "wait forever"; the others poll once); the wait is always bounded, never infinite.

61 +// On SPAWN_TIMEDWAIT_EXITED the instance has been freed, exactly like spawn_server_exec_wait().

62 +// On SPAWN_TIMEDWAIT_RUNNING or SPAWN_TIMEDWAIT_ERROR the caller keeps ownership and must eventually

63 +// call spawn_server_exec_timedwait(), spawn_server_exec_wait() or spawn_server_exec_kill().

64 +// ERROR is distinct from RUNNING on purpose: the wait could not progress (it is not merely "not yet"),

65 +// so callers must NOT loop on it (that would spin forever when timeout_ms == 0) - they must reclaim

66 +// the instance, typically by killing it.

67 +// status is optional (may be NULL): on EXITED the wait/cleanup still happens, the status is just not stored.

68 +SPAWN_TIMEDWAIT_RESULT spawn_server_exec_timedwait(SPAWN_SERVER *server, SPAWN_INSTANCE *si, int timeout_ms, int *status);

69 +

70 int spawn_server_instance_read_fd(SPAWN_INSTANCE *si);

71 int spawn_server_instance_write_fd(SPAWN_INSTANCE *si);

72 pid_t spawn_server_instance_pid(SPAWN_INSTANCE *si);

src/libnetdata/spawn_server/spawn_server_internals.h

@@ -8,6 +8,10 @@

8 #include "spawn_library.h"

9 #include "log-forwarder.h"

11 +// grace period spawn_server_exec_kill() waits after SIGTERM before escalating to SIGKILL,

12 +// used when the caller passes a non-positive timeout_ms (i.e. no explicit grace)

13 +#define SPAWN_KILL_DEFAULT_GRACE_MS 2000

14 +

15 #if defined(OS_WINDOWS)

16 #define SPAWN_SERVER_VERSION_WINDOWS 1

17 // #define SPAWN_SERVER_VERSION_UV 1

src/libnetdata/spawn_server/spawn_server_libuv.c

+39 -1

@@ -352,7 +352,7 @@ SPAWN_INSTANCE* spawn_server_exec(SPAWN_SERVER *server, int stderr_fd __maybe_un

352 return item.instance;

353 }

354

355 -int spawn_server_exec_kill(SPAWN_SERVER *server __maybe_unused, SPAWN_INSTANCE *si, int timeout_ms __maybe_unused) {

355 +int spawn_server_exec_kill(SPAWN_SERVER *server __maybe_unused, SPAWN_INSTANCE *si, int timeout_ms) {

356 if(!si) return -1;

357

358 // close all pipe descriptors to force the child to exit

@@ -364,9 +364,47 @@ int spawn_server_exec_kill(SPAWN_SERVER *server __maybe_unused, SPAWN_INSTANCE *

364 return -1;

365 }

366

367 + // escalate to SIGKILL if the child does not exit promptly after SIGTERM (or if the wait could

368 + // not be completed), so a SIGTERM-ignoring child cannot make the final wait block forever.

369 + // the caller's timeout_ms is the SIGTERM grace; fall back to a default when not specified.

370 + int grace_ms = timeout_ms > 0 ? timeout_ms : SPAWN_KILL_DEFAULT_GRACE_MS;

371 + int status;

372 + if(spawn_server_exec_timedwait(server, si, grace_ms, &status) != SPAWN_TIMEDWAIT_EXITED) {

373 + if(uv_process_kill(&si->process, SIGKILL))

374 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "SPAWN PARENT: uv_process_kill(SIGKILL) failed");

375 + }

376 + else

377 + return status;

378 +

379 return spawn_server_exec_wait(server, si);

380 }

381

382 +SPAWN_TIMEDWAIT_RESULT spawn_server_exec_timedwait(SPAWN_SERVER *server __maybe_unused, SPAWN_INSTANCE *si, int timeout_ms, int *status) {

383 + if (!si) { if(status) *status = -1; return SPAWN_TIMEDWAIT_EXITED; }

384 +

385 + // close all pipe descriptors to force the child to exit

386 + if(si->read_fd != -1) { close(si->read_fd); si->read_fd = -1; }

387 + if(si->write_fd != -1) { close(si->write_fd); si->write_fd = -1; }

388 +

389 + // a negative timeout would become a huge usec_t deadline (= unbounded wait); clamp to poll-once

390 + if(timeout_ms < 0) timeout_ms = 0;

391 + usec_t deadline_ut = now_monotonic_usec() + (usec_t)timeout_ms * USEC_PER_MS;

392 +

393 + while(uv_sem_trywait(&si->sem) != 0) {

394 + if(now_monotonic_usec() >= deadline_ut)

395 + return SPAWN_TIMEDWAIT_RUNNING;

396 +

397 + sleep_usec(10 * USEC_PER_MS);

398 + }

399 +

400 + // the semaphore is consumed - finish exactly like spawn_server_exec_wait()

401 + int st = si->exit_code;

402 + uv_sem_destroy(&si->sem);

403 + freez(si);

404 + if(status) *status = st;

405 + return SPAWN_TIMEDWAIT_EXITED;

406 +}

407 +

408 int spawn_server_exec_wait(SPAWN_SERVER *server __maybe_unused, SPAWN_INSTANCE *si) {

409 if (!si) return -1;

410

src/libnetdata/spawn_server/spawn_server_nofork.c

+69 -1

@@ -1206,6 +1206,44 @@ static void log_invalid_magic(SPAWN_INSTANCE *instance, struct status_report *sr

1206 instance->child_pid, instance->request_id, sr->magic, buf);

1207 }

1208

1209 +SPAWN_TIMEDWAIT_RESULT spawn_server_exec_timedwait(SPAWN_SERVER *server, SPAWN_INSTANCE *instance, int timeout_ms, int *status) {

1210 + if(!instance) { if(status) *status = -1; return SPAWN_TIMEDWAIT_EXITED; }

1211 +

1212 + // close the child pipes, to make it exit (same as spawn_server_exec_wait)

1213 + if(instance->write_fd != -1) { close(instance->write_fd); instance->write_fd = -1; }

1214 + if(instance->read_fd != -1) { close(instance->read_fd); instance->read_fd = -1; }

1215 +

1216 + // a non-positive timeout means "wait forever" to wait_on_socket_or_cancel_with_timeout();

1217 + // this primitive must always be bounded, so clamp to a minimal positive slice.

1218 + if(timeout_ms <= 0) timeout_ms = 1;

1219 +

1220 + // the spawn server sends the final status report on instance->sock when the child exits

1221 + short revents = 0;

1222 + NETDATA_SSL ssl = { 0 };

1223 + int rc = wait_on_socket_or_cancel_with_timeout(&ssl, instance->sock, timeout_ms, POLLIN, &revents);

1224 + if(rc == -1 /* thread cancelled */ || rc == 1 /* timeout */)

1225 + // the child is still running; the caller decides whether to keep waiting or kill it

1226 + return SPAWN_TIMEDWAIT_RUNNING;

1227 +

1228 + if(rc == 2 /* error on the socket */) {

1229 + // the status channel to the spawn server is broken (the spawn server itself died). We

1230 + // cannot confirm the child exited, so we must NOT resolve as EXITED and free the instance

1231 + // (that could leak a still-alive child). But this is terminal, not a transient "still

1232 + // running" state, so we must NOT report RUNNING either (a caller looping on RUNNING with a

1233 + // 0/"wait forever" timeout would spin forever). Report ERROR: the caller keeps the instance

1234 + // and reclaims it by killing it.

1235 + nd_log(NDLS_COLLECTORS, NDLP_ERR,

1236 + "SPAWN PARENT: status socket error for request No %zu, pid %d",

1237 + instance->request_id, instance->child_pid);

1238 + return SPAWN_TIMEDWAIT_ERROR;

1239 + }

1240 +

1241 + // rc == 0: the status report is ready to read; the blocking wait returns immediately now.

1242 + int st = spawn_server_exec_wait(server, instance);

1243 + if(status) *status = st;

1244 + return SPAWN_TIMEDWAIT_EXITED;

1245 +}

1246 +

1247 int spawn_server_exec_wait(SPAWN_SERVER *server __maybe_unused, SPAWN_INSTANCE *instance) {

1248 int rc = -1;

1249

@@ -1256,9 +1294,39 @@ int spawn_server_exec_kill(SPAWN_SERVER *server, SPAWN_INSTANCE *instance, int t

1294 }

1295

1296 // kill the child, if it is still running

1259 - if(instance->child_pid)

1297 + if(instance->child_pid) {

1298 kill(instance->child_pid, SIGTERM);

1299

1300 + // wait a bounded grace for the child to exit after SIGTERM. NOTE: timeout_ms is already

1301 + // consumed above as the pre-kill grace (voluntary exit before SIGTERM); the post-SIGTERM

1302 + // grace uses the fixed default so the caller's grace is not applied twice.

1303 + // No PID-reuse race on the RUNNING path: the spawn server reaps the child and only then

1304 + // sends the status report that makes timedwait return EXITED, so a RUNNING result means

1305 + // the child has not been reaped yet and its PID is still held.

1306 + int status;

1307 + if(spawn_server_exec_timedwait(server, instance, SPAWN_KILL_DEFAULT_GRACE_MS, &status) == SPAWN_TIMEDWAIT_EXITED)

1308 + return status;

1309 +

1310 + // still not gone: force-kill, then wait another bounded grace. We must NOT fall through to

1311 + // an unbounded blocking wait here - a child we cannot signal (e.g. SIGKILL returns EPERM)

1312 + // would otherwise hang the caller (and shutdown) forever, the very thing this path prevents.

1313 + if(kill(instance->child_pid, SIGKILL) != 0)

1314 + nd_log(NDLS_COLLECTORS, NDLP_ERR,

1315 + "SPAWN PARENT: SIGKILL of pid %d failed for request No %zu", instance->child_pid, instance->request_id);

1316 +

1317 + if(spawn_server_exec_timedwait(server, instance, SPAWN_KILL_DEFAULT_GRACE_MS, &status) == SPAWN_TIMEDWAIT_EXITED)

1318 + return status;

1319 +

1320 + // could not confirm the child exited within the bounded waits; reclaim the instance so we

1321 + // neither leak it nor block. The spawn server reaps the child if/when it actually dies.

1322 + nd_log(NDLS_COLLECTORS, NDLP_ERR,

1323 + "SPAWN PARENT: giving up waiting for pid %d after SIGKILL (request No %zu) - reclaiming",

1324 + instance->child_pid, instance->request_id);

1325 + instance->child_pid = 0; // already signalled; skip the SIGTERM in destroy

1326 + spawn_server_exec_destroy(instance);

1327 + return -1;

1328 + }

1329 +

1330 return spawn_server_exec_wait(server, instance);

1331 }

1332

src/libnetdata/spawn_server/spawn_server_posix.c

+57 -1

@@ -208,7 +208,7 @@ SPAWN_INSTANCE* spawn_server_exec(SPAWN_SERVER *server, int stderr_fd, int custo

208 return si;

209 }

210

211 -int spawn_server_exec_kill(SPAWN_SERVER *server, SPAWN_INSTANCE *si, int timeout_ms __maybe_unused) {

211 +int spawn_server_exec_kill(SPAWN_SERVER *server, SPAWN_INSTANCE *si, int timeout_ms) {

212 if (!si) return -1;

213

214 if (kill(si->child_pid, SIGTERM))

@@ -216,6 +216,23 @@ int spawn_server_exec_kill(SPAWN_SERVER *server, SPAWN_INSTANCE *si, int timeout

216 "SPAWN PARENT: kill() of pid %d failed: %s",

217 si->child_pid, si->cmdline);

218

219 + // escalate to SIGKILL if the child does not exit promptly after SIGTERM (or if the wait could

220 + // not be completed), so a SIGTERM-ignoring child cannot make the final wait block forever.

221 + // the caller's timeout_ms is the SIGTERM grace; fall back to a default when not specified.

222 + int grace_ms = timeout_ms > 0 ? timeout_ms : SPAWN_KILL_DEFAULT_GRACE_MS;

223 + int status;

224 + if(spawn_server_exec_timedwait(server, si, grace_ms, &status) != SPAWN_TIMEDWAIT_EXITED) {

225 + if(kill(si->child_pid, SIGKILL) != 0)

226 + // a failed SIGKILL almost always means the child is already gone (ESRCH); the wait

227 + // below then returns immediately. SIGKILL is uncatchable, so it cannot be ignored by

228 + // a live child - the only unbounded case left is uninterruptible (D-state) sleep,

229 + // which no signal or timeout can resolve.

230 + nd_log(NDLS_COLLECTORS, NDLP_ERR,

231 + "SPAWN PARENT: SIGKILL of pid %d failed: %s", si->child_pid, si->cmdline);

232 + }

233 + else

234 + return status;

235 +

236 return spawn_server_exec_wait(server, si);

237 }

238

@@ -270,6 +287,45 @@ static int spawn_server_waitpid(SPAWN_INSTANCE *si) {

287 return status;

288 }

289

290 +SPAWN_TIMEDWAIT_RESULT spawn_server_exec_timedwait(SPAWN_SERVER *server, SPAWN_INSTANCE *si, int timeout_ms, int *status) {

291 + if (!si) { if(status) *status = -1; return SPAWN_TIMEDWAIT_EXITED; }

292 +

293 + // close the child pipes to force it to exit, matching spawn_server_exec_wait and the

294 + // other backends; otherwise a child blocked on stdin/stdout would never see EOF and

295 + // would stay alive until the deadline forces a SIGKILL

296 + if (si->read_fd != -1) { close(si->read_fd); si->read_fd = -1; }

297 + if (si->write_fd != -1) { close(si->write_fd); si->write_fd = -1; }

298 +

299 + // a negative timeout would become a huge usec_t deadline (= unbounded wait); clamp to poll-once

300 + if(timeout_ms < 0) timeout_ms = 0;

301 + usec_t deadline_ut = now_monotonic_usec() + (usec_t)timeout_ms * USEC_PER_MS;

302 +

303 + while(!__atomic_load_n(&si->exited, __ATOMIC_RELAXED)) {

304 + int wstatus = 0;

305 + pid_t pid = waitpid(si->child_pid, &wstatus, WNOHANG);

306 + if(pid == si->child_pid) {

307 + __atomic_store_n(&si->waitpid_status, wstatus, __ATOMIC_RELAXED);

308 + __atomic_store_n(&si->exited, true, __ATOMIC_RELAXED);

309 + break;

310 + }

311 +

312 + if(pid < 0 && errno != EINTR)

313 + // child reaped elsewhere (e.g. ECHILD) - let the blocking wait resolve it immediately

314 + break;

315 +

316 + // pid == 0 (still running) or EINTR (interrupted before any state change):

317 + // keep waiting, but never past the deadline

318 + if(now_monotonic_usec() >= deadline_ut)

319 + return SPAWN_TIMEDWAIT_RUNNING;

320 +

321 + sleep_usec(10 * USEC_PER_MS);

322 + }

323 +

324 + int st = spawn_server_exec_wait(server, si);

325 + if(status) *status = st;

326 + return SPAWN_TIMEDWAIT_EXITED;

327 +}

328 +

329 int spawn_server_exec_wait(SPAWN_SERVER *server __maybe_unused, SPAWN_INSTANCE *si) {

330 if (!si) return -1;

331

src/libnetdata/spawn_server/spawn_server_windows.c

+31

@@ -431,6 +431,37 @@ int spawn_server_exec_kill(SPAWN_SERVER *server __maybe_unused, SPAWN_INSTANCE *

431 return spawn_server_exec_wait(server, si);

432 }

433

434 +SPAWN_TIMEDWAIT_RESULT spawn_server_exec_timedwait(SPAWN_SERVER *server, SPAWN_INSTANCE *si, int timeout_ms, int *status) {

435 + if(!si) { if(status) *status = -1; return SPAWN_TIMEDWAIT_EXITED; }

436 +

437 + if(si->read_fd != -1) { close(si->read_fd); si->read_fd = -1; }

438 + if(si->write_fd != -1) { close(si->write_fd); si->write_fd = -1; }

439 +

440 + // a negative timeout would become a huge DWORD (~INFINITE) to WaitForSingleObject; clamp to poll-once

441 + if(timeout_ms < 0) timeout_ms = 0;

442 +

443 + DWORD wait_rc = WaitForSingleObject(si->process_handle, (DWORD)timeout_ms);

444 + if(wait_rc == WAIT_TIMEOUT)

445 + // the process is still running; the caller decides whether to keep waiting or kill it

446 + return SPAWN_TIMEDWAIT_RUNNING;

447 +

448 + if(wait_rc == WAIT_FAILED) {

449 + // the handle is unusable, so we cannot confirm the process exited. We must NOT resolve as

450 + // EXITED and free the instance (it may still be alive), and we must NOT report RUNNING

451 + // either (a caller looping on RUNNING with a 0/"wait forever" timeout would spin forever).

452 + // Report ERROR: the caller keeps the instance and reclaims it by killing it.

453 + nd_log(NDLS_COLLECTORS, NDLP_ERR,

454 + "SPAWN PARENT: WaitForSingleObject() failed (err %lu) for request No %zu, pid %d (winpid %u)",

455 + (unsigned long)GetLastError(), si->request_id, (int)si->child_pid, si->dwProcessId);

456 + return SPAWN_TIMEDWAIT_ERROR;

457 + }

458 +

459 + // WAIT_OBJECT_0: the process exited; the blocking wait returns immediately now.

460 + int st = spawn_server_exec_wait(server, si);

461 + if(status) *status = st;

462 + return SPAWN_TIMEDWAIT_EXITED;

463 +}

464 +

465 int spawn_server_exec_wait(SPAWN_SERVER *server __maybe_unused, SPAWN_INSTANCE *si) {

466 if(si->read_fd != -1) { close(si->read_fd); si->read_fd = -1; }

467 if(si->write_fd != -1) { close(si->write_fd); si->write_fd = -1; }