Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deno.ChildProcess prevents Deno.exit even with .unref() #21662

Open
jtoppine opened this issue Dec 20, 2023 · 1 comment
Open

Deno.ChildProcess prevents Deno.exit even with .unref() #21662

jtoppine opened this issue Dec 20, 2023 · 1 comment
Labels
bug Something isn't working correctly

Comments

@jtoppine
Copy link

jtoppine commented Dec 20, 2023

Version: Deno 1.39.0 / Linux

From Deno.ChildProcess.unref() docs: Ensure that the status of the child process does not block the Deno process from exiting.

This seems to work in case of the parent process coming naturally to end (nothing to do). It also works when parent process encounters unhandled exception or rejection. But if parent process calls Deno.exit(), deno appears to wait for the child process to end before actually exiting, even if unref() was called on it.

Encountered this while researching another subprocess related issue so this reproduction may have some extra stuff and is not exactly minimal. Anyway, you'll see how "exit-with-deno-exit" cases take about two seconds instead of returning immediately like the other test cases (child process self regulates if lifetime to max two seconds which is enough to verify behaviour).

Or is this intentional behaviour?

// needs to be saved and run as "subprocesstest.js"
const mode = Deno.args[0] || "runtests";

// -------------- RUN TESTS

if (mode == "runtests") {
  const runtest = async (exitmode, killsignal) => {
    const starttime = Date.now();
    console.log(exitmode);
    const command = new Deno.Command(Deno.execPath(), {
      args: [
        "run",
        "--allow-run",
        "--allow-read",
        "subprocesstest.js",
        "parent-process",
        exitmode,
      ],
      stderr: "piped",
      stdin: "piped",
      stdout: "piped",
    });
    const child = command.spawn();

    if (killsignal) {
      setTimeout(() => {
        console.log("- sending kill signal " + killsignal);
        child.kill(killsignal);
      }, 100);
    }

    const output = await child.output();
    const stdout = new TextDecoder().decode(output.stdout);

    const pid = stdout.split(":")[1];

    // console.log("- child pid: " + pid);

    const checkcommand = new Deno.Command("ps", {
      args: ["-p", pid],
      stderr: "piped",
      stdin: "piped",
      stdout: "piped",
    });
    const checkchild = checkcommand.spawn();
    const checkoutput = await checkchild.output();
    const checkstdout = new TextDecoder().decode(checkoutput.stdout);
    const checkstatus = checkstdout.split("\n")[1]?.split(" ")[2];

    if (checkstatus == pid) {
      console.log("- child is still alive (should not happen)");
    } else {
      console.log("- child not found (good)");
    }
    const endtime = Date.now();
    if (endtime - starttime > 1000) {
      console.log(
        "- test took too long, did parent process wait for child to exit?",
      );
    }
  };

  await runtest("exit-naturally", null);
  await runtest("exit-with-error", null);
  await runtest("exit-with-unhandled-rejection", null);
  await runtest("exit-with-deno-exit-0", null);
  await runtest("exit-with-deno-exit-1", null);

  await runtest("let-parent-handle-kill", "SIGTERM");
  await runtest("let-parent-handle-kill", "SIGKILL");
  await runtest("let-parent-handle-kill", "SIGINT");
}

// -------------- PARENT PROCESS MODE

if (mode == "parent-process") {
  const exitmode = Deno.args[1];
  const command = new Deno.Command(Deno.execPath(), {
    args: [
      "run",
      "--allow-run",
      "--allow-read",
      "--allow-net",
      "subprocesstest.js",
      "long-running-child",
    ],
    stderr: "inherit",
    stdin: "piped",
    stdout: "inherit",
  });
  const child = command.spawn();

  // need to unref here to
  child.unref();

  // communicate the child process id to the parent process
  console.log("PID:" + child.pid + ":");

  setTimeout(() => {
    if (exitmode == "exit-naturally") {
      // no-op
    }
    if (exitmode == "exit-with-error") {
      throw new Error("exit-with-error");
    }
    if (exitmode == "exit-with-unhandled-rejection") {
      Promise.reject("exit-with-unhandled-rejection");
    }
    if (exitmode == "exit-with-deno-exit-0") {
      Deno.exit(0);
    }
    if (exitmode == "exit-with-deno-exit-1") {
      Deno.exit(1);
    }
    if (exitmode == "let-parent-handle-kill") {
      setTimeout(() => {
        // wait for a while to allow parent time to send kill signal
      }, 1000);
    }
  }, 10);
}

// -------------- CHILD PROCESS MODE

if (mode == "long-running-child") {
  console.log("long running");
  setTimeout(() => {
    // do nothing for two seconds to give time for tests to run but not too long in case it gets stuck
  }, 2000);
}

Note that the repro assumes it is saved as subprocesstest.js so that when it is run with deno run -A subprocesstest.js, it can run itself in a different "mode".

It outputs this:

exit-naturally
- child not found (good)
exit-with-error
- child not found (good)
exit-with-unhandled-error
- child not found (good)
exit-with-deno-exit-0
- child not found (good)
- test took too long, did parent process wait for child to exit?
exit-with-deno-exit-1
- child not found (good)
- test took too long, did parent process wait for child to exit?

The expected output would be the same but without the - test took too long, did parent process wait for child to exit?lines.

@mmastrac mmastrac added the bug Something isn't working correctly label Dec 21, 2023
@jtoppine
Copy link
Author

Small update:

  • Updated (edited) the repro above to also include tests for process.kill(), just for completeness. It seems process.kill()from the parent has the same effect as Deno.exit()from inside the child.

  • I had brief discussion a couple of days earlier on discord with @marvinhagemeister , about an issue where it seems child processes are left inadvertedly alive after the parent process dies (acting like a detached process, effectively). I promised to create an issue about that. However, I'm starting to suspect that might not be a separate issue at all, but that this unref() business described here is the root of both problems. I can only verify that after this issue is sorted out :)

  • There is a separate feature request to support explicit detachedprocesses, where it is actually desireable to let spawned childs to outlive the main process. One issue about that ( Detached processes in deno #5501 ) has some discussion that may be relevant to this issue. Just mentioning here to link these loosely related things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

2 participants