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.

3 files changed, 120 insertions, 52 deletions

diff --git a/chime.el b/chime.el
index 66190c8..52ed569 100644
--- a/chime.el
+++ b/chime.el
@@ -914,6 +914,14 @@ you theme it."
 Consulted by the watchdog in `chime--fetch-and-process' to detect a
 child that exceeded `chime-async-timeout'.")
 
+(defvar chime--process-generation 0
+  "Counter identifying the current async child.
+Incremented on every spawn, and on every event that abandons a child (the
+watchdog interrupt, and `chime--stop').  Each child's callback captures the
+generation it was spawned under and compares it here, so a straggler that
+returns after its child was abandoned can be discarded rather than clobber
+the replacement child's state.")
+
 (defvar chime--consecutive-async-failures 0
   "Count of consecutive async check failures.
 After `chime-max-consecutive-failures' failures, a warning is displayed.")
@@ -2381,12 +2389,18 @@ When called programmatically, returns structured validation results."
 ;;;; Core Lifecycle
 
 (defun chime--stop ()
-  "Stop the notification timer and cancel any in-progress check."
+  "Stop the notification timer and cancel any in-progress check.
+Leaves no state behind: a later `chime-mode' must not resume with the old
+failure count, a stale spawn time, or a callback from the child this stop
+abandoned."
   (-some-> chime--timer (cancel-timer))
   (setq chime--timer nil)
-  (when chime--process
-    (interrupt-process chime--process)
-    (setq chime--process nil))
+  (chime--kill-async-process chime--process)
+  (setq chime--process nil)
+  (setq chime--process-start-time nil)
+  ;; Orphan the abandoned child's callback, if it is still in flight.
+  (cl-incf chime--process-generation)
+  (setq chime--consecutive-async-failures 0)
   ;; Reset validation state so it runs again on next start
   (setq chime--validation-done nil)
   (setq chime--validation-retry-count 0))
@@ -2479,15 +2493,40 @@ deprecated per-event property."
   (chime--maybe-warn-deprecated-properties events)
   (funcall callback events))
 
+(defun chime--kill-async-process (process)
+  "Kill PROCESS and reap the buffer async.el leaves behind.
+Does nothing when PROCESS is nil or already dead.
+
+`interrupt-process' only asks: a child stuck in a blocking read ignores
+SIGINT and survives as a zombie, invisible because `chime--process' has
+already been cleared.  `delete-process' does not ask.
+
+async.el's sentinel kills the child's process buffer only on a zero exit,
+so a signalled child leaks its buffer.  A persistent hang leaks one per
+`chime-async-timeout' until Emacs restarts, which is why the buffer is
+killed here rather than left to async.  The sentinel is silenced first so
+async cannot act on a child we have already abandoned."
+  (when (processp process)
+    (let ((buffer (process-buffer process)))
+      (set-process-sentinel process #'ignore)
+      (when (process-live-p process)
+        (delete-process process))
+      (when (buffer-live-p buffer)
+        (kill-buffer buffer)))))
+
 (defun chime--interrupt-stale-process ()
-  "Interrupt an async child that has outlived `chime-async-timeout'.
+  "Kill an async child that has outlived `chime-async-timeout'.
 A child that never returns (e.g. stuck on an interactive prompt in batch
 mode) is invisible to every failure path — it returns no error sexp, so
 `chime--consecutive-async-failures' never increments while the overlap
-guard silently skips every subsequent check.  Interrupting it and
-recording the timeout through `chime--record-async-failure' routes the
-hang into the existing failure machinery and frees the next check to
-spawn a fresh child.  Does nothing when `chime-async-timeout' is nil."
+guard silently skips every subsequent check.  Killing it and recording the
+timeout through `chime--record-async-failure' routes the hang into the
+existing failure machinery and frees the next check to spawn a fresh
+child.  Does nothing when `chime-async-timeout' is nil.
+
+Bumping `chime--process-generation' orphans the abandoned child's
+callback, so a straggler that returns just past the timeout cannot clobber
+the replacement child's state."
   (when (and chime-async-timeout
              chime--process
              (process-live-p chime--process)
@@ -2495,38 +2534,54 @@ spawn a fresh child.  Does nothing when `chime-async-timeout' is nil."
              (> (float-time (time-subtract (current-time)
                                            chime--process-start-time))
                 chime-async-timeout))
-    (interrupt-process chime--process)
+    (chime--kill-async-process chime--process)
     (setq chime--process nil)
+    (setq chime--process-start-time nil)
+    (cl-incf chime--process-generation)
     (chime--record-async-failure
      (list 'error (format "Async fetch exceeded chime-async-timeout (%ds)"
                           chime-async-timeout))
      "Async watchdog")))
 
+(defun chime--handle-async-result (generation callback events)
+  "Handle EVENTS returned by the async child spawned under GENERATION.
+CALLBACK receives the events on success.
+
+Discards the result when GENERATION is no longer current — the child was
+abandoned by the watchdog or by `chime--stop', and a replacement may
+already be running.  Acting on a straggler would clear the replacement's
+process handle (breaking the overlap guard, so a third child could spawn)
+and reset the failure counter the watchdog had just incremented."
+  (when (= generation chime--process-generation)
+    (setq chime--process nil)
+    (setq chime--process-start-time nil)
+    (setq chime--last-check-time (current-time))
+    (condition-case err
+        (if (and (listp events)
+                 (eq (car events) 'async-signal))
+            (chime--record-async-failure (cdr events) "Async error")
+          (chime--handle-async-success callback events))
+      (error
+       (chime--record-async-failure err "Error processing events")))))
+
 (defun chime--fetch-and-process (callback)
   "Asynchronously fetch events from agenda and invoke CALLBACK with them.
 Manages async process state and last-check-time internally.
 Does nothing if a check is already in progress, unless that check has
-exceeded `chime-async-timeout' — then it is interrupted and replaced."
+exceeded `chime-async-timeout' — then it is killed and replaced."
   (chime--interrupt-stale-process)
   (unless (and chime--process
                (process-live-p chime--process))
     (setq chime--process-start-time (current-time))
-    (setq chime--process
-          (let ((default-directory user-emacs-directory)
-                (async-prompt-for-password nil)
-                (async-process-noquery-on-exit t))
-            (async-start
-             (chime--retrieve-events)
-             (lambda (events)
-               (setq chime--process nil)
-               (setq chime--last-check-time (current-time))
-               (condition-case err
-                   (if (and (listp events)
-                            (eq (car events) 'async-signal))
-                       (chime--record-async-failure (cdr events) "Async error")
-                     (chime--handle-async-success callback events))
-                 (error
-                  (chime--record-async-failure err "Error processing events")))))))))
+    (let ((generation (cl-incf chime--process-generation)))
+      (setq chime--process
+            (let ((default-directory user-emacs-directory)
+                  (async-prompt-for-password nil)
+                  (async-process-noquery-on-exit t))
+              (async-start
+               (chime--retrieve-events)
+               (lambda (events)
+                 (chime--handle-async-result generation callback events))))))))
 
 (defun chime--log-silently (format-string &rest args)
   "Append formatted message to *Messages* buffer without echoing.
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))))