Skip to content

Commit

Permalink
fix: Child.unref() unrefs stdio streams properly (denoland#15275)
Browse files Browse the repository at this point in the history
  • Loading branch information
bartlomieju committed Jul 22, 2022
1 parent 4db650d commit 7219930
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 2 deletions.
29 changes: 27 additions & 2 deletions cli/tests/unit/spawn_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -723,8 +723,29 @@ Deno.test(
const program = `
const child = await Deno.spawnChild(Deno.execPath(), {
cwd: Deno.args[0],
stdout: "piped",
args: ["run", "-A", "--unstable", Deno.args[1]],
});
const readable = child.stdout.pipeThrough(new TextDecoderStream());
const reader = readable.getReader();
// set up an interval that will end after reading a few messages from stdout,
// to verify that stdio streams are properly unrefed
let count = 0;
let interval;
interval = setInterval(async () => {
count += 1;
if (count > 10) {
clearInterval(interval);
console.log("cleared interval");
}
const res = await reader.read();
if (res.done) {
throw new Error("stream shouldn't be done");
}
if (res.value.trim() != "hello from interval") {
throw new Error("invalid message received");
}
}, 120);
console.log("spawned pid", child.pid);
child.unref();
`;
Expand All @@ -740,15 +761,19 @@ setInterval(() => {
// In this subprocess we are spawning another subprocess which has
// an infite interval set. Following call would never resolve unless
// child process gets unrefed.
const { success, stdout } = await Deno.spawn(Deno.execPath(), {
const { success, stdout, stderr } = await Deno.spawn(Deno.execPath(), {
cwd,
args: ["run", "-A", "--unstable", programFile, cwd, childProgramFile],
});

assert(success);
const stdoutText = new TextDecoder().decode(stdout);
const pidStr = stdoutText.split(" ").at(-1);
const stderrText = new TextDecoder().decode(stderr);
assert(stderrText.length == 0);
const [line1, line2] = stdoutText.split("\n");
const pidStr = line1.split(" ").at(-1);
assert(pidStr);
assertEquals(line2, "cleared interval");
const pid = Number.parseInt(pidStr, 10);
await Deno.remove(cwd, { recursive: true });
// Child process should have been killed when parent process exits.
Expand Down
5 changes: 5 additions & 0 deletions runtime/js/40_spawn.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
class Child {
#rid;
#waitPromiseId;
#unrefed = false;

#pid;
get pid() {
Expand Down Expand Up @@ -132,13 +133,15 @@
this.#stdoutRid = stdoutRid;
this.#stdout = readableStreamForRid(stdoutRid, (promise) => {
this.#stdoutPromiseId = promise[promiseIdSymbol];
if (this.#unrefed) core.unrefOp(this.#stdoutPromiseId);
});
}

if (stderrRid !== null) {
this.#stderrRid = stderrRid;
this.#stderr = readableStreamForRid(stderrRid, (promise) => {
this.#stderrPromiseId = promise[promiseIdSymbol];
if (this.#unrefed) core.unrefOp(this.#stderrPromiseId);
});
}

Expand Down Expand Up @@ -204,12 +207,14 @@
}

ref() {
this.#unrefed = false;
core.refOp(this.#waitPromiseId);
if (this.#stdoutPromiseId) core.refOp(this.#stdoutPromiseId);
if (this.#stderrPromiseId) core.refOp(this.#stderrPromiseId);
}

unref() {
this.#unrefed = true;
core.unrefOp(this.#waitPromiseId);
if (this.#stdoutPromiseId) core.unrefOp(this.#stdoutPromiseId);
if (this.#stderrPromiseId) core.unrefOp(this.#stderrPromiseId);
Expand Down

0 comments on commit 7219930

Please sign in to comment.