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

feat(cli): enable sub-process to run in detached mode #7481

Closed
wants to merge 2 commits into from
Closed

feat(cli): enable sub-process to run in detached mode #7481

wants to merge 2 commits into from

Conversation

notfilippo
Copy link

@notfilippo notfilippo commented Sep 14, 2020

The PR is to enable processes started via Deno.run() to run in detached mode.

This is a continuation of #5836,
Discussion can be found in #5501

TODO:

  • Typing checks to ensure that piped is not passed to a detached process
  • Check windows output from the CI and figure out how to test it

@CLAassistant
Copy link

CLAassistant commented Sep 14, 2020

CLA assistant check
All committers have signed the CLA.

@notfilippo notfilippo marked this pull request as draft September 14, 2020 23:23
@notfilippo
Copy link
Author

Should I introduce this as an unstable feature?

@notfilippo
Copy link
Author

There are still some issues with windows not properly detaching... it seems like process spawner hangs until child exits even if detached flags are provided.

I based my windows code on this part of the rust std:
https://github.com/rust-lang/rust/blob/6cae28165f0450fd3f100374b26841e458b8cfef/library/std/src/sys/windows/process.rs#L171-L174

@lucacasonato
Copy link
Member

@bartlomieju
Copy link
Member

@Qu4k please rebase the PR and let's try to get it landed for 1.6.0

@notfilippo
Copy link
Author

@Qu4k please rebase the PR and let's try to get it landed for 1.6.0

I can but this still doesn't work in windows...

@notfilippo notfilippo marked this pull request as ready for review November 22, 2020 20:04
@stale
Copy link

stale bot commented Feb 1, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 1, 2021
@lirannl
Copy link

lirannl commented Feb 5, 2021

Has this been abandoned? I don't really use Windows so I can't help, but I feel like it's pretty important to be able to spawn detached child processes.

@stale stale bot removed the stale label Feb 5, 2021
@notfilippo
Copy link
Author

Has this been abandoned? I don't really use Windows so I can't help, but I feel like it's pretty important to be able to spawn detached child processes.

Well it's not abandoned but i don't have any way to test on windows so i'm blocked

Base automatically changed from master to main February 19, 2021 14:58
@piscisaureus piscisaureus self-assigned this Feb 24, 2021
@stale
Copy link

stale bot commented Apr 25, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 25, 2021
@bartlomieju
Copy link
Member

@filipporeds thank you for the PR, but I'm going to close it without a merge. Currently it's most likely that we'll adjust subprocess API as described in #9435 and ability to detach processes will be included in the rewrite. Thanks again!

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

Successfully merging this pull request may close these issues.

8 participants