fix(imessage): clear stop timeout after child closes (#108909)

* fix(imessage): clear stop timeout after child closes

* test(imessage): preserve timer spy types

* test(imessage): preserve writable end return contract

* docs(imessage): explain stop timer cleanup

Co-authored-by: zhang-guiping <zhang.guiping@xydigit.com>

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>

extensions/imessage/src/client.test.ts

@@ -86,6 +86,38 @@ describe("IMessageRpcClient child stream error handling", () => {
     },
   );
 
+  it("clears the stop fallback timer when the child closes first", async () => {
+    const realClearTimeout = globalThis.clearTimeout;
+    const setTimeoutSpy = vi.spyOn(globalThis, "setTimeout");
+    const clearTimeoutSpy = vi.spyOn(globalThis, "clearTimeout");
+    let scheduledTimer: NodeJS.Timeout | undefined;
+
+    try {
+      const { IMessageRpcClient } = await import("./client.js");
+      const client = new IMessageRpcClient({ cliPath: "imsg" });
+      await client.start();
+      const endMock = vi.fn(() => {
+        child.emit("close", 0, null);
+        return child.stdin;
+      });
+      child.stdin.end = endMock;
+
+      await client.stop();
+
+      expect(endMock).toHaveReturnedWith(child.stdin);
+      expect(setTimeoutSpy).toHaveBeenCalledOnce();
+      scheduledTimer = setTimeoutSpy.mock.results[0]?.value as NodeJS.Timeout | undefined;
+      expect(scheduledTimer).toBeDefined();
+      expect(clearTimeoutSpy).toHaveBeenCalledWith(scheduledTimer);
+      expect(child.kill).not.toHaveBeenCalled();
+    } finally {
+      if (scheduledTimer) {
+        realClearTimeout(scheduledTimer);
+      }
+      vi.restoreAllMocks();
+    }
+  });
+
   it("settles the client after a real child stdout stream failure", async () => {
     const childProcess =
       await vi.importActual<typeof import("node:child_process")>("node:child_process");

extensions/imessage/src/client.ts

@@ -156,17 +156,26 @@ export class IMessageRpcClient {
     const child = this.child;
     this.child = null;
 
-    await Promise.race([
-      this.closed,
-      new Promise<void>((resolve) => {
-        setTimeout(() => {
-          if (!child.killed) {
-            child.kill("SIGTERM");
-          }
-          resolve();
-        }, 500);
-      }),
-    ]);
+    let timeout: ReturnType<typeof setTimeout> | undefined;
+    try {
+      await Promise.race([
+        this.closed,
+        new Promise<void>((resolve) => {
+          timeout = setTimeout(() => {
+            if (!child.killed) {
+              child.kill("SIGTERM");
+            }
+            resolve();
+          }, 500);
+        }),
+      ]);
+    } finally {
+      // A losing fallback still holds Node's event loop open; clear it when
+      // the child closes first so short-lived RPC clients can exit promptly.
+      if (timeout) {
+        clearTimeout(timeout);
+      }
+    }
   }
 
   async waitForClose(): Promise<void> {