Skip to content

Commit 0e48239

Browse files
tarastaras
committed
Add explicit kill help clean up
1 parent df2ee37 commit 0e48239

File tree

4 files changed

+151
-122
lines changed

4 files changed

+151
-122
lines changed

‎process/src/exec/api.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ export interface Process extends StdIO {
2424
* not complete successfully, it will raise an ExecError.
2525
*/
2626
expect(): Operation<ExitStatus>;
27+
28+
/**
29+
* Kill the child process
30+
*/
31+
kill(): Operation<void>;
2732
}
2833

2934
export interface ExecOptions {
@@ -77,6 +82,7 @@ export interface ProcessResult extends ExitStatus {
7782
stdout: string;
7883
stderr: string;
7984
}
85+
8086
export interface CreateOSProcess {
8187
(command: string, options: ExecOptions): Operation<Process>;
8288
}

‎process/src/exec/posix.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,18 +90,21 @@ export const createPosixProcess: CreateOSProcess = function* createPosixProcess(
9090
processResult.resolve(Ok(value));
9191
} finally {
9292
try {
93-
if (typeof childProcess.pid === "undefined") {
94-
// deno-lint-ignore no-unsafe-finally
95-
throw new Error("no pid for childProcess");
96-
}
97-
process.kill(-childProcess.pid, "SIGTERM");
98-
yield* all([io.stdoutDone.operation, io.stderrDone.operation]);
93+
yield* kill();
9994
} catch (_e) {
10095
// do nothing, process is probably already dead
10196
}
10297
}
10398
});
10499

100+
function* kill() {
101+
if (typeof childProcess.pid === "undefined") {
102+
throw new Error("no pid for childProcess");
103+
}
104+
process.kill(-childProcess.pid, "SIGTERM");
105+
yield* all([io.stdoutDone.operation, io.stderrDone.operation]);
106+
}
107+
105108
function* join() {
106109
let result = yield* processResult.operation;
107110
if (result.ok) {
@@ -129,5 +132,6 @@ export const createPosixProcess: CreateOSProcess = function* createPosixProcess(
129132
stderr,
130133
join,
131134
expect,
135+
kill,
132136
};
133137
};

‎process/src/exec/win32.ts‎

Lines changed: 58 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -108,71 +108,74 @@ export const createWin32Process: CreateOSProcess = function* createWin32Process(
108108
processResult.resolve(Ok(value));
109109
} finally {
110110
try {
111-
// Only try to kill the process if it hasn't exited yet
112-
if (
113-
childProcess.exitCode === null &&
114-
childProcess.signalCode === null
115-
) {
116-
if (typeof childProcess.pid === "undefined") {
117-
// deno-lint-ignore no-unsafe-finally
118-
throw new Error("no pid for childProcess");
119-
}
111+
yield* kill();
112+
} catch (_e) {
113+
// do nothing, process is probably already dead
114+
}
115+
}
116+
});
120117

121-
let stdinStream = childProcess.stdin;
118+
function* kill() {
119+
// Only try to kill the process if it hasn't exited yet
120+
if (
121+
childProcess.exitCode === null &&
122+
childProcess.signalCode === null
123+
) {
124+
if (typeof childProcess.pid === "undefined") {
125+
throw new Error("no pid for childProcess");
126+
}
122127

123-
// Try graceful shutdown with ctrlc
124-
try {
125-
ctrlc(childProcess.pid);
126-
if (stdinStream.writable) {
127-
try {
128-
// Terminate batch process (Y/N)
129-
stdinStream.write("Y\n");
130-
} catch (_err) {
131-
// not much we can do here
132-
}
133-
}
134-
} catch (_err) {
135-
// ctrlc might fail
136-
}
128+
let stdinStream = childProcess.stdin;
137129

138-
// Close stdin to allow process to exit cleanly
130+
// Try graceful shutdown with ctrlc
131+
try {
132+
ctrlc(childProcess.pid);
133+
if (stdinStream.writable) {
139134
try {
140-
stdinStream.end();
135+
// Terminate batch process (Y/N)
136+
stdinStream.write("Y\n");
141137
} catch (_err) {
142-
// stdin might already be closed
138+
// not much we can do here
143139
}
140+
}
141+
} catch (_err) {
142+
// ctrlc might fail
143+
}
144144

145-
// If process still hasn't exited, escalate
146-
if (
147-
childProcess.exitCode === null &&
148-
childProcess.signalCode === null
149-
) {
150-
// Try regular kill first
151-
try {
152-
childProcess.kill();
153-
} catch (_err) {
154-
// process might already be dead
155-
}
156-
157-
// If still alive after kill, force-kill entire process tree
158-
// This is necessary for bash on Windows where ctrlc doesn't work
159-
// and child.kill() only kills the shell, leaving grandchildren alive
160-
if (
161-
childProcess.exitCode === null &&
162-
childProcess.signalCode === null
163-
) {
164-
yield* killTree(childProcess.pid);
165-
}
166-
}
145+
// Close stdin to allow process to exit cleanly
146+
try {
147+
stdinStream.end();
148+
} catch (_err) {
149+
// stdin might already be closed
150+
}
167151

168-
// Wait for streams to finish
169-
yield* all([io.stdoutDone.operation, io.stderrDone.operation]);
152+
// If process still hasn't exited, escalate
153+
if (
154+
childProcess.exitCode === null &&
155+
childProcess.signalCode === null
156+
) {
157+
// Try regular kill first
158+
try {
159+
childProcess.kill();
160+
} catch (_err) {
161+
// process might already be dead
162+
}
163+
164+
// If still alive after kill, force-kill entire process tree
165+
// This is necessary for bash on Windows where ctrlc doesn't work
166+
// and child.kill() only kills the shell, leaving grandchildren alive
167+
if (
168+
childProcess.exitCode === null &&
169+
childProcess.signalCode === null
170+
) {
171+
yield* killTree(childProcess.pid);
170172
}
171-
} catch (_e) {
172-
// do nothing, process is probably already dead
173173
}
174+
175+
// Wait for streams to finish
176+
yield* all([io.stdoutDone.operation, io.stderrDone.operation]);
174177
}
175-
});
178+
}
176179

177180
function* join() {
178181
let result = yield* processResult.operation;
@@ -201,6 +204,7 @@ export const createWin32Process: CreateOSProcess = function* createWin32Process(
201204
stderr,
202205
join,
203206
expect,
207+
kill,
204208
};
205209
};
206210

‎watch/test/helpers.ts‎

Lines changed: 77 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { assert } from "@std/assert";
33
import { emptyDir, ensureDir } from "@std/fs";
44
import { dirname, fromFileUrl, join } from "@std/path";
55
import type { Operation, Result, Stream } from "effection";
6-
import { each, Ok, sleep, spawn, until } from "effection";
6+
import { all, each, Ok, resource, sleep, spawn, until } from "effection";
77
import { cp, readFile, writeFile } from "node:fs/promises";
88

99
import type { Start } from "../watch.ts";
@@ -67,72 +67,87 @@ type SuccessfulStart = {
6767

6868
type ProcessStart = Result<SuccessfulStart>;
6969

70-
export function* inspector(stream: Stream<Start, never>) {
71-
let starts: ProcessStart[] = [];
72-
73-
let expected = 0;
70+
interface Inspector {
71+
starts: ProcessStart[];
72+
expectNext(): Operation<SuccessfulStart>;
73+
expectNoRestart(): Operation<void>;
74+
}
7475

75-
yield* spawn(function* () {
76-
for (let { result } of yield* each(stream)) {
77-
if (result.ok) {
78-
let process = result.value;
79-
let start = {
80-
stdout: "",
81-
stderr: "",
82-
process: result.value,
83-
};
84-
starts.push(Ok(start));
85-
yield* spawn(function* () {
86-
for (let chunk of yield* each(process.stdout)) {
87-
start.stdout += String(chunk);
88-
yield* each.next();
89-
}
90-
});
91-
yield* spawn(function* () {
92-
for (let chunk of yield* each(process.stderr)) {
93-
start.stderr += String(chunk);
94-
yield* each.next();
95-
}
96-
});
97-
} else {
98-
starts.push(result);
99-
}
76+
export function inspector(stream: Stream<Start, never>): Operation<Inspector> {
77+
return resource(function* (provide) {
78+
let starts: ProcessStart[] = [];
10079

101-
yield* each.next();
102-
}
103-
});
80+
let expected = 0;
10481

105-
let inspector = {
106-
starts,
107-
*expectNext(): Operation<SuccessfulStart> {
108-
let initial = expected;
109-
for (let i = 0; i < 500; i++) {
110-
if (initial < starts.length) {
111-
yield* sleep(10);
112-
expected = starts.length;
113-
let result = inspector.starts[inspector.starts.length - 1];
114-
if (result.ok) {
115-
return result.value;
116-
} else {
117-
throw new Error(
118-
`expected successful start, but failed: ${result.error}`,
119-
);
120-
}
82+
yield* spawn(function* () {
83+
for (let { result } of yield* each(stream)) {
84+
if (result.ok) {
85+
let process = result.value;
86+
let start = {
87+
stdout: "",
88+
stderr: "",
89+
process: result.value,
90+
};
91+
starts.push(Ok(start));
92+
yield* spawn(function* () {
93+
for (let chunk of yield* each(process.stdout)) {
94+
start.stdout += String(chunk);
95+
yield* each.next();
96+
}
97+
});
98+
yield* spawn(function* () {
99+
for (let chunk of yield* each(process.stderr)) {
100+
start.stderr += String(chunk);
101+
yield* each.next();
102+
}
103+
});
121104
} else {
122-
yield* sleep(10);
105+
starts.push(result);
123106
}
107+
108+
yield* each.next();
124109
}
125-
throw new Error(`expecting a sucessful start but it never appeared.`);
126-
},
127-
*expectNoRestart() {
128-
let prexisting = inspector.starts.length;
129-
yield* sleep(200);
130-
let restarts = inspector.starts.length - prexisting;
131-
assert(
132-
restarts === 0,
133-
`expected no process restarts to have happened, but instead there were: ${restarts}`,
110+
});
111+
112+
try {
113+
yield* provide({
114+
starts,
115+
*expectNext() {
116+
let initial = expected;
117+
for (let i = 0; i < 500; i++) {
118+
if (initial < starts.length) {
119+
yield* sleep(10);
120+
expected = starts.length;
121+
let result = starts[starts.length - 1];
122+
if (result.ok) {
123+
return result.value;
124+
} else {
125+
throw new Error(
126+
`expected successful start, but failed: ${result.error}`,
127+
);
128+
}
129+
} else {
130+
yield* sleep(10);
131+
}
132+
}
133+
throw new Error(`expecting a sucessful start but it never appeared.`);
134+
},
135+
*expectNoRestart() {
136+
let prexisting = starts.length;
137+
yield* sleep(200);
138+
let restarts = starts.length - prexisting;
139+
assert(
140+
restarts === 0,
141+
`expected no process restarts to have happened, but instead there were: ${restarts}`,
142+
);
143+
},
144+
});
145+
} finally {
146+
yield* all(
147+
starts.filter((start) => start.ok).map((start) =>
148+
start.value.process.kill()
149+
),
134150
);
135-
},
136-
};
137-
return inspector;
151+
}
152+
});
138153
}

0 commit comments

Comments
 (0)