From 132947e67fecfc7698426b8a05e2f5f6dca21ca0 Mon Sep 17 00:00:00 2001
From: Taras Mankovski <74687+taras@users.noreply.github.com>
Date: Mon, 10 Aug 2026 02:19:56 -0400
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Arm=20process=20kill-teardown=20?=
 =?UTF-8?q?before=20spawning=20the=20child?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A halt delivered while exec was still acquiring its process could land
on any suspension point between the OS spawn and the ensure that
registers the kill-teardown, discarding the rest of the acquisition
sequence. The teardown never armed, so the child — and its whole
process tree — kept running with no owner.

Register a guard teardown before the process exists and spawn in the
same synchronous continuation, so at no point does the process exist
without an armed teardown. The guard reuses the configured shutdown
semantics but joins on process exit alone, since the stdio pumps may
never have been wired; it stands down once the primary teardown at the
end of the acquisition has begun. Exit, close, and error observation
moves from spawned watchers to Node listeners attached in the spawn's
continuation so the guard can always join on process exit, wherever a
halt lands.

The regression test exploits the scheduler's round-robin between
same-generation routines to sweep a halt deterministically across
every suspension point of the acquisition sequence.

Fixes #236
---
 process/src/exec/posix.ts | 198 +++++++++++++++------------
 process/src/exec/win32.ts | 280 ++++++++++++++++++++++----------------
 process/test/exec.test.ts |  64 +++++++++
 3 files changed, 335 insertions(+), 207 deletions(-)

diff --git a/process/src/exec/posix.ts b/process/src/exec/posix.ts
index 474c0ad7..6fee72e4 100644
--- a/process/src/exec/posix.ts
+++ b/process/src/exec/posix.ts
@@ -1,4 +1,4 @@
-import { spawn as spawnProcess } from "node:child_process";
+import { type ChildProcess, spawn as spawnProcess } from "node:child_process";
 import process from "node:process";
 import {
   type Result,
@@ -14,7 +14,6 @@ import {
   withResolvers,
 } from "effection";
 import { unbox, useEvalScope } from "@effectionx/scope-eval";
-import { once } from "@effectionx/node/events";
 import { fromReadable } from "@effectionx/node/stream";
 import type {
   CreateOSProcess,
@@ -38,6 +37,92 @@ export function* createPosixProcess(
   let processResult = withResolvers<Result<ProcessResultValue>>();
   const evalScope = yield* useEvalScope();
   const result = yield* evalScope.eval(function* () {
+    let stdoutDone = withResolvers<void>();
+    let stderrDone = withResolvers<void>();
+
+    let childProcess: ChildProcess | undefined;
+    let teardownStarted = false;
+
+    const shutdown = options.shutdown ?? "graceful";
+
+    const exit: Operation<ExitStatus> = {
+      *[Symbol.iterator]() {
+        let result = yield* exitResult.operation;
+        if (result.ok) {
+          let [code, signal] = result.value;
+          return { command, options, code, signal } as ExitStatus;
+        }
+        throw result.error;
+      },
+    };
+
+    function* terminate(): Operation<void> {
+      let pid = childProcess?.pid;
+      if (typeof pid !== "undefined") {
+        try {
+          process.kill(-pid, "SIGKILL");
+        } catch (_error) {}
+      }
+    }
+
+    function* reaped(): Operation<void> {
+      yield* processResult.operation;
+    }
+
+    function* closed(): Operation<void> {
+      yield* all([
+        processResult.operation,
+        stdoutDone.operation,
+        stderrDone.operation,
+      ]);
+    }
+
+    function* shutdownProcess(join: () => Operation<void>): Operation<void> {
+      let pid = childProcess?.pid;
+      if (typeof pid === "undefined") {
+        return;
+      }
+
+      function* gracefulCompletion(): Operation<ShutdownMode> {
+        yield* join();
+        return "graceful";
+      }
+
+      if (shutdown === "forced") {
+        yield* terminate();
+      } else {
+        try {
+          process.kill(-pid, "SIGTERM");
+        } catch (_error) {}
+
+        if (typeof shutdown === "function") {
+          let mode: ShutdownMode;
+          try {
+            mode = yield* race([gracefulCompletion(), shutdown({ exit })]);
+          } catch (_error) {
+            mode = "forced";
+          }
+          if (mode === "forced") {
+            yield* terminate();
+          }
+        }
+      }
+
+      yield* settled(join());
+    }
+
+    // A halt can land on any suspension point between here and the primary
+    // teardown at the end of this generator, discarding every instruction
+    // after it. This guard registers while `childProcess` is still empty and
+    // the spawn below follows in the same synchronous continuation, so at no
+    // point does the process exist without an armed teardown. It joins on
+    // process exit alone because the stdio pumps may never have been wired.
+    yield* ensure(function* () {
+      if (!teardownStarted) {
+        yield* shutdownProcess(reaped);
+      }
+    });
+
     // Killing all child processes started by this command is surprisingly
     // tricky. If a process spawns another processes and we kill the parent,
     // then the child process is NOT automatically killed. Instead we're using
@@ -48,25 +133,38 @@ export function* createPosixProcess(
     // process.
     //
     // More information here: https://unix.stackexchange.com/questions/14815/process-descendants
-    let childProcess = spawnProcess(command, options.arguments || [], {
+    const child = spawnProcess(command, options.arguments || [], {
       detached: true,
       shell: options.shell,
       env: options.env,
       cwd: options.cwd,
       stdio: "pipe",
     });
+    childProcess = child;
 
-    let { pid } = childProcess;
+    // Node listeners instead of spawned effection watchers: exit observation
+    // must arm in the same synchronous continuation as the spawn so that the
+    // guard above can join on it no matter where a halt lands.
+    child.once("error", (error) => {
+      exitResult.resolve(Err(error));
+      processResult.resolve(Err(error));
+    });
+    child.once("exit", (code, signal) => {
+      exitResult.resolve(Ok([code ?? undefined, signal ?? undefined]));
+    });
+    child.once("close", (code, signal) => {
+      processResult.resolve(Ok([code ?? undefined, signal ?? undefined]));
+    });
 
-    if (!childProcess.stdout || !childProcess.stderr) {
+    let { pid } = child;
+
+    if (!child.stdout || !child.stderr) {
       throw new Error("stdout and stderr must be available with stdio: pipe");
     }
 
     let io = {
-      stdout: yield* fromReadable(childProcess.stdout),
-      stderr: yield* fromReadable(childProcess.stderr),
-      stdoutDone: withResolvers<void>(),
-      stderrDone: withResolvers<void>(),
+      stdout: yield* fromReadable(child.stdout),
+      stderr: yield* fromReadable(child.stderr),
     };
 
     let stdout = createSignal<Uint8Array, void>();
@@ -80,7 +178,7 @@ export function* createPosixProcess(
         next = yield* io.stdout.next();
       }
       stdout.close();
-      io.stdoutDone.resolve();
+      stdoutDone.resolve();
     });
 
     yield* spawn(function* () {
@@ -91,39 +189,12 @@ export function* createPosixProcess(
         next = yield* io.stderr.next();
       }
       stderr.close();
-      io.stderrDone.resolve();
+      stderrDone.resolve();
     });
 
     let stdin: Writable<string> = {
       send(data: string) {
-        childProcess.stdin.write(data);
-      },
-    };
-
-    yield* spawn(function* trapError() {
-      let [error] = yield* once<[Error]>(childProcess, "error");
-      exitResult.resolve(Err(error));
-      processResult.resolve(Err(error));
-    });
-
-    yield* spawn(function* () {
-      let value = yield* once<ProcessResultValue>(childProcess, "exit");
-      exitResult.resolve(Ok(value));
-    });
-
-    yield* spawn(function* () {
-      let value = yield* once<ProcessResultValue>(childProcess, "close");
-      processResult.resolve(Ok(value));
-    });
-
-    const exit: Operation<ExitStatus> = {
-      *[Symbol.iterator]() {
-        let result = yield* exitResult.operation;
-        if (result.ok) {
-          let [code, signal] = result.value;
-          return { command, options, code, signal } as ExitStatus;
-        }
-        throw result.error;
+        child.stdin.write(data);
       },
     };
 
@@ -144,54 +215,9 @@ export function* createPosixProcess(
       return status;
     }
 
-    function* closed(): Operation<void> {
-      yield* all([
-        processResult.operation,
-        io.stdoutDone.operation,
-        io.stderrDone.operation,
-      ]);
-    }
-
-    function* gracefulCompletion(): Operation<ShutdownMode> {
-      yield* closed();
-      return "graceful";
-    }
-
-    function* terminate(): Operation<void> {
-      if (typeof pid !== "undefined") {
-        try {
-          process.kill(-pid, "SIGKILL");
-        } catch (_error) {}
-      }
-    }
-
-    const shutdown = options.shutdown ?? "graceful";
-
     yield* ensure(function* () {
-      if (shutdown === "forced") {
-        yield* terminate();
-      } else {
-        try {
-          if (typeof pid === "undefined") {
-            throw new Error("no pid for childProcess");
-          }
-          process.kill(-pid, "SIGTERM");
-        } catch (_error) {}
-
-        if (typeof shutdown === "function") {
-          let mode: ShutdownMode;
-          try {
-            mode = yield* race([gracefulCompletion(), shutdown({ exit })]);
-          } catch (_error) {
-            mode = "forced";
-          }
-          if (mode === "forced") {
-            yield* terminate();
-          }
-        }
-      }
-
-      yield* settled(closed());
+      teardownStarted = true;
+      yield* shutdownProcess(closed);
     });
 
     return {
diff --git a/process/src/exec/win32.ts b/process/src/exec/win32.ts
index 6d78b0b8..6220223f 100644
--- a/process/src/exec/win32.ts
+++ b/process/src/exec/win32.ts
@@ -1,3 +1,4 @@
+import type { ChildProcess } from "node:child_process";
 import { platform } from "node:os";
 import { once } from "@effectionx/node/events";
 import { fromReadable } from "@effectionx/node/stream";
@@ -53,8 +54,123 @@ export function* createWin32Process(
   let processResult = withResolvers<Result<ProcessResultValue>>();
   const evalScope = yield* useEvalScope();
   const result = yield* evalScope.eval(function* () {
+    let stdoutDone = withResolvers<void>();
+    let stderrDone = withResolvers<void>();
+    let rawClose = withResolvers<Result<ProcessResultValue>>();
+
+    let childProcess: ChildProcess | undefined;
+    let teardownStarted = false;
+
+    const shutdown = options.shutdown ?? "graceful";
+
+    let hardTerminationRequested = false;
+    const hardTerminationRequest = withResolvers<void>();
+
+    function requestHardTermination(): void {
+      if (!hardTerminationRequested) {
+        hardTerminationRequested = true;
+        hardTerminationRequest.resolve();
+      }
+    }
+
+    const exit: Operation<ExitStatus> = {
+      *[Symbol.iterator]() {
+        let result = yield* exitResult.operation;
+        if (result.ok) {
+          let [code, signal] = result.value;
+          return { command, options, code, signal } as ExitStatus;
+        }
+        throw result.error;
+      },
+    };
+
+    function* reaped(): Operation<void> {
+      yield* rawClose.operation;
+    }
+
+    function* closed(): Operation<void> {
+      yield* all([
+        processResult.operation,
+        stdoutDone.operation,
+        stderrDone.operation,
+      ]);
+    }
+
+    function* shutdownProcess(join: () => Operation<void>): Operation<void> {
+      let child = childProcess;
+      let pid = child?.pid;
+      if (!child || typeof pid === "undefined") {
+        return;
+      }
+
+      function* gracefulCompletion(): Operation<ShutdownMode> {
+        yield* join();
+        return "graceful";
+      }
+
+      const hardTerminationTask =
+        shutdown !== "graceful"
+          ? yield* spawn(function* () {
+              yield* hardTerminationRequest.operation;
+              yield* killTree(pid);
+            })
+          : undefined;
+
+      if (shutdown === "forced") {
+        requestHardTermination();
+      } else {
+        try {
+          ctrlc(pid);
+        } catch (_) {}
+
+        let stdin = child.stdin;
+        if (stdin) {
+          if (stdin.writable) {
+            try {
+              stdin.write("Y\n");
+            } catch (_error) {}
+          }
+          stdin.end();
+        }
+
+        if (typeof shutdown === "function") {
+          let mode: ShutdownMode;
+          try {
+            mode = yield* race([gracefulCompletion(), shutdown({ exit })]);
+          } catch (_error) {
+            mode = "forced";
+          }
+          if (mode === "forced") {
+            requestHardTermination();
+          }
+        }
+      }
+
+      if (hardTerminationTask) {
+        if (hardTerminationRequested) {
+          yield* hardTerminationTask;
+        } else {
+          yield* hardTerminationTask.halt();
+        }
+      }
+
+      yield* settled(join());
+    }
+
+    // A halt can land on any suspension point between here and the primary
+    // teardown at the end of this generator, discarding every instruction
+    // after it. This guard registers while `childProcess` is still empty and
+    // the spawn below follows in the same synchronous continuation, so at no
+    // point does the process exist without an armed teardown. It joins on
+    // process exit alone because the stdio pumps may never have been wired.
+    yield* ensure(function* () {
+      if (!teardownStarted) {
+        yield* shutdownProcess(reaped);
+      }
+    });
+
     // Windows-specific process spawning with different options than POSIX
-    let childProcess = spawnProcess(command, options.arguments || [], {
+    const child = spawnProcess(command, options.arguments || [], {
       // We lose exit information and events if this is detached in windows
       // and it opens a window in windows+powershell.
       detached: false,
@@ -73,18 +189,41 @@ export function* createWin32Process(
       env: options.env,
       cwd: options.cwd,
     });
+    childProcess = child;
+
+    // Node listeners instead of spawned effection watchers: exit observation
+    // must arm in the same synchronous continuation as the spawn so that the
+    // guard above can join on it no matter where a halt lands.
+    child.once("error", (error) => {
+      exitResult.resolve(Err(error));
+      processResult.resolve(Err(error));
+      rawClose.resolve(Err(error));
+    });
+    child.once("exit", (code, signal) => {
+      exitResult.resolve(Ok([code ?? undefined, signal ?? undefined]));
+    });
+    child.once("close", (code, signal) => {
+      rawClose.resolve(Ok([code ?? undefined, signal ?? undefined]));
+    });
+
+    // Suppress EPIPE errors on stdin - these occur on Windows when the child
+    // process exits before we finish writing to it. This is expected during
+    // cleanup when we're killing the process.
+    child.stdin.on("error", (err: Error & { code?: string }) => {
+      if (err.code !== "EPIPE") {
+        throw err;
+      }
+    });
 
-    let { pid } = childProcess;
+    let { pid } = child;
 
-    if (!childProcess.stdout || !childProcess.stderr) {
+    if (!child.stdout || !child.stderr) {
       throw new Error("stdout and stderr must be available with stdio: pipe");
     }
 
     let io = {
-      stdout: yield* fromReadable(childProcess.stdout),
-      stderr: yield* fromReadable(childProcess.stderr),
-      stdoutDone: withResolvers<void>(),
-      stderrDone: withResolvers<void>(),
+      stdout: yield* fromReadable(child.stdout),
+      stderr: yield* fromReadable(child.stderr),
     };
 
     const stdout = createSignal<Uint8Array, void>();
@@ -98,7 +237,7 @@ export function* createWin32Process(
         next = yield* io.stdout.next();
       }
       stdout.close();
-      io.stdoutDone.resolve();
+      stdoutDone.resolve();
     });
 
     yield* spawn(function* () {
@@ -109,46 +248,26 @@ export function* createWin32Process(
         next = yield* io.stderr.next();
       }
       stderr.close();
-      io.stderrDone.resolve();
+      stderrDone.resolve();
     });
 
     let stdin: Writable<string> = {
       send(data: string) {
-        childProcess.stdin.write(data);
+        child.stdin.write(data);
       },
     };
 
-    yield* spawn(function* trapError() {
-      const [error] = yield* once<Error[]>(childProcess, "error");
-      exitResult.resolve(Err(error));
-      processResult.resolve(Err(error));
-    });
-
-    yield* spawn(function* () {
-      let value = yield* once<ProcessResultValue>(childProcess, "exit");
-      exitResult.resolve(Ok(value));
-    });
-
     yield* spawn(function* () {
-      let value = yield* once<ProcessResultValue>(childProcess, "close");
-      // out of band with the finally block below compared to posix as
-      // win32 is more sensitive to graceful shutdown timing that it is
-      // worth waiting for stdout and stderr to close before resolving the process result
-      yield* all([io.stdoutDone.operation, io.stderrDone.operation]);
-      processResult.resolve(Ok(value));
+      let result = yield* rawClose.operation;
+      if (result.ok) {
+        // win32 is more sensitive to graceful shutdown timing than posix, so
+        // it is worth waiting for stdout and stderr to close before resolving
+        // the process result
+        yield* all([stdoutDone.operation, stderrDone.operation]);
+      }
+      processResult.resolve(result);
     });
 
-    const exit: Operation<ExitStatus> = {
-      *[Symbol.iterator]() {
-        let result = yield* exitResult.operation;
-        if (result.ok) {
-          let [code, signal] = result.value;
-          return { command, options, code, signal } as ExitStatus;
-        }
-        throw result.error;
-      },
-    };
-
     function* join() {
       let result = yield* processResult.operation;
       if (result.ok) {
@@ -166,90 +285,9 @@ export function* createWin32Process(
       return status;
     }
 
-    function* closed(): Operation<void> {
-      yield* all([
-        processResult.operation,
-        io.stdoutDone.operation,
-        io.stderrDone.operation,
-      ]);
-    }
-
-    function* gracefulCompletion(): Operation<ShutdownMode> {
-      yield* closed();
-      return "graceful";
-    }
-
-    let hardTerminationRequested = false;
-    const hardTerminationRequest = withResolvers<void>();
-
-    function requestHardTermination(): void {
-      if (!hardTerminationRequested) {
-        hardTerminationRequested = true;
-        hardTerminationRequest.resolve();
-      }
-    }
-
-    const shutdown = options.shutdown ?? "graceful";
-
-    // Suppress EPIPE errors on stdin - these occur on Windows when the child
-    // process exits before we finish writing to it. This is expected during
-    // cleanup when we're killing the process.
-    childProcess.stdin.on("error", (err: Error & { code?: string }) => {
-      if (err.code !== "EPIPE") {
-        throw err;
-      }
-    });
-
     yield* ensure(function* () {
-      const hardTerminationTask =
-        shutdown !== "graceful"
-          ? yield* spawn(function* () {
-              yield* hardTerminationRequest.operation;
-              if (pid) {
-                yield* killTree(pid);
-              }
-            })
-          : undefined;
-
-      if (shutdown === "forced") {
-        requestHardTermination();
-      } else {
-        if (pid) {
-          try {
-            ctrlc(pid);
-          } catch (_) {}
-
-          let stdin = childProcess.stdin;
-          if (stdin.writable) {
-            try {
-              stdin.write("Y\n");
-            } catch (_error) {}
-          }
-          stdin.end();
-        }
-
-        if (typeof shutdown === "function") {
-          let mode: ShutdownMode;
-          try {
-            mode = yield* race([gracefulCompletion(), shutdown({ exit })]);
-          } catch (_error) {
-            mode = "forced";
-          }
-          if (mode === "forced") {
-            requestHardTermination();
-          }
-        }
-      }
-
-      if (hardTerminationTask) {
-        if (hardTerminationRequested) {
-          yield* hardTerminationTask;
-        } else {
-          yield* hardTerminationTask.halt();
-        }
-      }
-
-      yield* settled(closed());
+      teardownStarted = true;
+      yield* shutdownProcess(closed);
     });
 
     return {
diff --git a/process/test/exec.test.ts b/process/test/exec.test.ts
index 4b7fb97a..51d2cbbb 100644
--- a/process/test/exec.test.ts
+++ b/process/test/exec.test.ts
@@ -1,11 +1,14 @@
+import { execSync } from "node:child_process";
 import process from "node:process";
 import { beforeEach, describe, it } from "@effectionx/vitest";
 import {
+  type Operation,
   type Task,
   createContext,
   sleep,
   spawn,
   suspend,
+  useScope,
   withResolvers,
 } from "effection";
 import { expect } from "expect";
@@ -774,4 +777,65 @@ describe("exec", () => {
       });
     }
   });
+
+  if (process.platform !== "win32") {
+    describe("halt during acquisition", () => {
+      function deep(
+        levels: number,
+        body: () => Operation<void>,
+      ): Operation<void> {
+        if (levels === 0) {
+          return body();
+        }
+        return {
+          *[Symbol.iterator]() {
+            const inner = yield* spawn(() => deep(levels - 1, body));
+            yield* inner;
+          },
+        };
+      }
+
+      function isRunning(marker: string): boolean {
+        const commands = execSync("ps -axww -o command").toString();
+        return commands
+          .split("\n")
+          .some((line) => line.includes(marker) && !line.includes("ps -axww"));
+      }
+
+      it("never leaves the child process behind, wherever the halt lands", function* () {
+        // The scheduler interleaves same-generation routines one
+        // instruction per turn, so a halter spawned one scope deeper than
+        // the exec task runs in lockstep with the acquisition sequence.
+        // Sweeping the number of turns before the halt lands it on every
+        // suspension point of that sequence — including between the OS
+        // spawn and the registration of the kill-teardown (#236).
+        for (let turns = 0; turns < 40; turns++) {
+          const marker = `halt-sweep-236-${process.pid}-${turns}`;
+          const task = yield* spawn(function* () {
+            yield* exec("node", {
+              arguments: ["-e", "setTimeout(() => {}, 30000)", marker],
+            });
+            yield* suspend();
+          });
+          const halter = yield* spawn(() =>
+            deep(1, function* () {
+              for (let t = 0; t < turns; t++) {
+                yield* useScope();
+              }
+              yield* task.halt();
+            }),
+          );
+          yield* halter;
+          if (isRunning(marker)) {
+            try {
+              execSync(`pkill -f ${marker}`);
+            } catch (_error) {}
+            throw new Error(
+              `process orphaned when halted after ${turns} scheduler turns`,
+            );
+          }
+        }
+      }, 30000);
+    });
+  }
 });