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

Add deliver_cancel parameter and start support to run_process #673

Open
1 task done
gschaffner opened this issue Jan 17, 2024 · 0 comments
Open
1 task done

Add deliver_cancel parameter and start support to run_process #673

gschaffner opened this issue Jan 17, 2024 · 0 comments
Labels
enhancement New feature or request

Comments

@gschaffner
Copy link
Collaborator

gschaffner commented Jan 17, 2024

Things to check first

  • I have searched the existing issues and didn't find my feature already requested there

Feature description

Trio

  1. has a deliver_cancel parameter and nursery.start support in run_process
  2. changed their examples to suggest that folks use process = await nursery.start(run_process, ...) instead of open_process.
  3. moved open_process to lowlevel
  4. removed Process.aclose

Use case

  • process = await nursery.start(trio.run_process, ...) actually enforces structured concurrency—you can't accidentally leave an orphan process1
  • open_process does not enforce structured concurrency, hence it should be in lowlevel. while you can make it enforce structured concurrency by doing async with await trio.lowlevel.open_process() as process, process = await trio.lowlevel.open_process() by itself does not use structured concurrency and is basically fork() without a finally: wait(pid) to follow it. hence i lean toward agreeing that open_process should be in lowlevel.

i'd like to propose doing (1) and (2) in AnyIO: adding deliver_cancel support and TaskGroup.start support to run_process. i actually ended implementing this some months ago in a downstream AnyIO application at work, because upon cancellation we needed to send the child a gentle SIGINT first instead of reaching straight for SIGKILL and causing data loss. i'd be happy to move that implementation upstream if it seems reasonable.

if proposals (1) and (2) are accepted, AnyIO could also consider doing (3) and/or (4). they also make sense to me, as mentioned above—put open_process in lowlevel so users think twice before shooting themselves in the foot with the gun-that-orphans-processes.

thoughts?

Footnotes

  1. except if the parent gets SIGKILL/TerminateProcess and can't run any except/finally/__exit__/__aexit__/atexit code.

@gschaffner gschaffner added the enhancement New feature or request label Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant