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>(); const evalScope = yield* useEvalScope(); const result = yield* evalScope.eval(function* () { + let stdoutDone = withResolvers(); + let stderrDone = withResolvers(); + + let childProcess: ChildProcess | undefined; + let teardownStarted = false; + + const shutdown = options.shutdown ?? "graceful"; + + const exit: Operation = { + *[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 { + let pid = childProcess?.pid; + if (typeof pid !== "undefined") { + try { + process.kill(-pid, "SIGKILL"); + } catch (_error) {} + } + } + + function* reaped(): Operation { + yield* processResult.operation; + } + + function* closed(): Operation { + yield* all([ + processResult.operation, + stdoutDone.operation, + stderrDone.operation, + ]); + } + + function* shutdownProcess(join: () => Operation): Operation { + let pid = childProcess?.pid; + if (typeof pid === "undefined") { + return; + } + + function* gracefulCompletion(): Operation { + 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(), - stderrDone: withResolvers(), + stdout: yield* fromReadable(child.stdout), + stderr: yield* fromReadable(child.stderr), }; let stdout = createSignal(); @@ -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 = { 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(childProcess, "exit"); - exitResult.resolve(Ok(value)); - }); - - yield* spawn(function* () { - let value = yield* once(childProcess, "close"); - processResult.resolve(Ok(value)); - }); - - const exit: Operation = { - *[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 { - yield* all([ - processResult.operation, - io.stdoutDone.operation, - io.stderrDone.operation, - ]); - } - - function* gracefulCompletion(): Operation { - yield* closed(); - return "graceful"; - } - - function* terminate(): Operation { - 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>(); const evalScope = yield* useEvalScope(); const result = yield* evalScope.eval(function* () { + let stdoutDone = withResolvers(); + let stderrDone = withResolvers(); + let rawClose = withResolvers>(); + + let childProcess: ChildProcess | undefined; + let teardownStarted = false; + + const shutdown = options.shutdown ?? "graceful"; + + let hardTerminationRequested = false; + const hardTerminationRequest = withResolvers(); + + function requestHardTermination(): void { + if (!hardTerminationRequested) { + hardTerminationRequested = true; + hardTerminationRequest.resolve(); + } + } + + const exit: Operation = { + *[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 { + yield* rawClose.operation; + } + + function* closed(): Operation { + yield* all([ + processResult.operation, + stdoutDone.operation, + stderrDone.operation, + ]); + } + + function* shutdownProcess(join: () => Operation): Operation { + let child = childProcess; + let pid = child?.pid; + if (!child || typeof pid === "undefined") { + return; + } + + function* gracefulCompletion(): Operation { + 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(), - stderrDone: withResolvers(), + stdout: yield* fromReadable(child.stdout), + stderr: yield* fromReadable(child.stderr), }; const stdout = createSignal(); @@ -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 = { send(data: string) { - childProcess.stdin.write(data); + child.stdin.write(data); }, }; - yield* spawn(function* trapError() { - const [error] = yield* once(childProcess, "error"); - exitResult.resolve(Err(error)); - processResult.resolve(Err(error)); - }); - - yield* spawn(function* () { - let value = yield* once(childProcess, "exit"); - exitResult.resolve(Ok(value)); - }); - yield* spawn(function* () { - let value = yield* once(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 = { - *[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 { - yield* all([ - processResult.operation, - io.stdoutDone.operation, - io.stderrDone.operation, - ]); - } - - function* gracefulCompletion(): Operation { - yield* closed(); - return "graceful"; - } - - let hardTerminationRequested = false; - const hardTerminationRequest = withResolvers(); - - 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, + ): Operation { + 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); + }); + } });