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

Function to kill a running process #1494

Closed
hayd opened this issue Jan 10, 2019 · 8 comments · Fixed by #2177
Closed

Function to kill a running process #1494

hayd opened this issue Jan 10, 2019 · 8 comments · Fixed by #2177

Comments

@hayd
Copy link
Contributor

hayd commented Jan 10, 2019

There doesn't seem to be a way to kill a process initiated by run.
close only removes it from the resources list.
A running process continues after the original deno application is complete.

A few options:

  1. Kill all running/open processes when deno is exited
  2. Close should kill the process.
  3. There should be a new function to kill a process.

I favor 1+2.

@ry
Copy link
Member

ry commented Jan 10, 2019

Close ought to kill it. And #1 should be true too. I think these are expected behavior.
Thanks for reporting.

@hayd
Copy link
Contributor Author

hayd commented Jan 12, 2019

  1. Doesn't appear to be the case, example:
import { run } from "deno";

const justAMinute = run({
  args: ["python", "-c", "from time import sleep; sleep(60);"]
});

(async () => {
  // 5 seconds
  await new Promise(res => setTimeout(res, 5000));
  // justAMinute.close();
})();

// python is still running

This stemmed from this close in the deno_std test not cleaning up the file_server. Which I can also still replicate.... Could this be that the .close is not waiting for the response? It's weird as in the example above adding the close does work as expected.
If I add an await for a second after this line (in test.ts) it does clean up for me.

Note/Aside: If you kill -9 an application, e.g. python, they usually won't kill of their child processes (I assume this is a unix standard)...

@hayd
Copy link
Contributor Author

hayd commented Jan 15, 2019

I wonder if std::process::exit is the issue and we're not calling all the destructors.

Note that because this function never returns, and that it terminates the process, no destructors on the current stack or any other thread's stack will be run. If a clean shutdown is needed it is recommended to only call this function at a known point where there are no more destructors left to run.

From the documentation it reads like this ought to be dropped (and closed).


This seems similar to #1517 cc @kevinkassimo ....

@ry
Copy link
Member

ry commented Jan 15, 2019

It’s likely that we need to create a “process group” and stick all child processes in there (it’s a posix thing).

@kevinkassimo
Copy link
Contributor

@hayd I'll also investigate. Thanks for pinging me

@bartlomieju
Copy link
Member

@hayd any update on this?

@kevinkassimo
Copy link
Contributor

Sorry... Working on this (seems easy enough to fix)

@kevinkassimo
Copy link
Contributor

Hmm I have just tested, it seems that the example @hayd provides is working now with latest release...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants