From e43e44cb4cef93b3e3ae68396a0af639cea1534f Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Thu, 9 Jul 2026 19:58:27 -0500 Subject: fix: kill abandoned async children and discard their late callbacks Three faults in one path, all made reachable by the watchdog. A callback returning just past the timeout ran against state that no longer belonged to it. It nil'd chime--process, which by then held the replacement child, so the overlap guard broke and a third child could spawn. It also reset the failure counter the watchdog had just incremented. Each spawn now captures a generation, and a result whose generation is stale is discarded whole. interrupt-process only asks. A child stuck in a blocking read ignores SIGINT and lives on as a zombie, invisible because chime--process was already cleared. The watchdog and chime--stop now call chime--kill-async-process, which silences the sentinel and calls delete-process. async.el kills a child's process buffer only on a zero exit, so every signalled child leaked one. With a persistent hang that's a buffer per chime-async-timeout until Emacs restarts. The kill path reaps the buffer itself. chime--stop also clears chime--consecutive-async-failures and chime--process-start-time, so a later chime-mode doesn't resume with the old count. --- tests/test-chime-async-watchdog.el | 32 +++++++++++++++++++------------- tests/test-chime-lifecycle.el | 31 +++++++++++++++++++------------ 2 files changed, 38 insertions(+), 25 deletions(-) (limited to 'tests') diff --git a/tests/test-chime-async-watchdog.el b/tests/test-chime-async-watchdog.el index 473131d..f1b9829 100644 --- a/tests/test-chime-async-watchdog.el +++ b/tests/test-chime-async-watchdog.el @@ -86,34 +86,40 @@ Interruption requires strictly exceeding the timeout." (should-not interrupted) (should-not spawned))) -(ert-deftest test-chime-fetch-and-process-stale-child-interrupted-and-respawned () - "Error: an over-age child is interrupted, recorded as a failure, replaced. +(ert-deftest test-chime-fetch-and-process-stale-child-killed-and-respawned () + "Error: an over-age child is killed, recorded as a failure, replaced. The hung child must feed the existing consecutive-failures machinery (via `chime--record-async-failure') instead of silently blocking every tick, and -the same tick spawns a fresh child." +the same tick spawns a fresh child. + +The child is killed rather than interrupted: SIGINT is a request a child +stuck in a blocking read can ignore." (let* ((now (current-time)) - (interrupted nil) + (killed nil) (recorded nil) (spawned nil) (chime--process 'fake-live-process) (chime--process-start-time (time-subtract now (seconds-to-time 121))) + (chime--process-generation 0) (chime-async-timeout 120)) (cl-letf (((symbol-function 'current-time) (lambda () now)) ((symbol-function 'process-live-p) (lambda (proc) (eq proc 'fake-live-process))) - ((symbol-function 'interrupt-process) - (lambda (proc) (setq interrupted proc))) + ((symbol-function 'chime--kill-async-process) + (lambda (proc) (setq killed proc))) ((symbol-function 'chime--record-async-failure) (lambda (err prefix) (setq recorded (cons prefix err)))) ((symbol-function 'async-start) (lambda (&rest _) (setq spawned t) 'new-fake-process))) - (chime--fetch-and-process (lambda (_events) nil))) - (should (eq 'fake-live-process interrupted)) - (should (equal "Async watchdog" (car recorded))) - (should (string-match-p "chime-async-timeout" - (error-message-string (cdr recorded)))) - (should spawned) - (should (eq 'new-fake-process chime--process)))) + (chime--fetch-and-process (lambda (_events) nil)) + (should (eq 'fake-live-process killed)) + (should (equal "Async watchdog" (car recorded))) + (should (string-match-p "chime-async-timeout" + (error-message-string (cdr recorded)))) + (should spawned) + (should (eq 'new-fake-process chime--process)) + ;; One bump for abandoning the stale child, one for the replacement. + (should (= chime--process-generation 2))))) (ert-deftest test-chime-fetch-and-process-nil-timeout-disables-watchdog () "Boundary: `chime-async-timeout' nil disables the watchdog entirely. diff --git a/tests/test-chime-lifecycle.el b/tests/test-chime-lifecycle.el index 8ce9c13..826de4c 100644 --- a/tests/test-chime-lifecycle.el +++ b/tests/test-chime-lifecycle.el @@ -21,32 +21,39 @@ (require 'test-bootstrap (expand-file-name "test-bootstrap.el")) (require 'cl-lib) -(ert-deftest test-chime-stop-interrupts-running-process () - "Normal: when chime--process is set, `chime--stop' interrupts it and clears the var." - (let ((interrupted nil) +(ert-deftest test-chime-stop-kills-running-process () + "Normal: when chime--process is set, `chime--stop' kills it and clears the var. +The child is killed rather than interrupted, because SIGINT is a request a +child stuck in a blocking read can ignore." + (let ((killed nil) (chime--timer nil) (chime--process 'fake-process) + (chime--process-generation 0) (chime--validation-done t) (chime--validation-retry-count 5)) - (cl-letf (((symbol-function 'interrupt-process) - (lambda (proc) (setq interrupted proc)))) + (cl-letf (((symbol-function 'chime--kill-async-process) + (lambda (proc) (setq killed proc)))) (chime--stop)) - (should (eq 'fake-process interrupted)) + (should (eq 'fake-process killed)) (should (null chime--process)) + ;; The abandoned child's callback is orphaned. + (should (= 1 chime--process-generation)) (should-not chime--validation-done) (should (= 0 chime--validation-retry-count)))) -(ert-deftest test-chime-stop-no-process-skips-interrupt () - "Boundary: with chime--process nil, `interrupt-process' is never called." - (let ((interrupted nil) +(ert-deftest test-chime-stop-no-process-kills-nothing () + "Boundary: with chime--process nil, no process is killed." + (let ((killed 'untouched) (chime--timer nil) (chime--process nil) + (chime--process-generation 0) (chime--validation-done t) (chime--validation-retry-count 5)) - (cl-letf (((symbol-function 'interrupt-process) - (lambda (proc) (setq interrupted proc)))) + (cl-letf (((symbol-function 'chime--kill-async-process) + (lambda (proc) (setq killed proc)))) (chime--stop)) - (should (null interrupted)) + ;; chime--stop calls the helper unconditionally; it must tolerate nil. + (should (null killed)) (should-not chime--validation-done) (should (= 0 chime--validation-retry-count)))) -- cgit v1.2.3