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

calling create without file stdio blocks #31

Open
tonistiigi opened this issue Nov 29, 2017 · 2 comments
Open

calling create without file stdio blocks #31

tonistiigi opened this issue Nov 29, 2017 · 2 comments

Comments

@tonistiigi
Copy link
Member

os/exec allows 2 ways to wait on command: Cmd.Wait() makes sure all the goroutines that can be created by Cmd.Start() are cleaned up and doesn't return before stdio is closed. Process.Wait() returns after the process has exited but leaks goroutines. Previously monitor.Wait() in this package waited only for process but was changed to fix those leaks.

Problem is that there is one command Create() that by design takes stdio, forwards it to its children and then exits itself. That means that Cmd.Wait() does not return because the attach stdio has not finished. In containerd this does not appear because the interfaces passed in are always regular files(fifos) and don't create extra goroutines in os/exec internals.

I'd recommend either changing CreateOpt to take *io.File or implementing pipe copy in this library so it can be cleaned up outside of stdlib(that would probably mean adding a cleanup function to the IO interface).

@crosbymichael

@crosbymichael
Copy link
Member

Misread on the first pass.

Isn't this by design and how exec.Cmd has always worked?

@tonistiigi
Copy link
Member Author

What part is by design? exec.Cmd.Wait() isn't necessarily doing anything wrong in here but it can't be used in a way runc.Create does because exec.Cmd counts stdio as part of the command and runc.Create has different lifecycle for the command and the io streams.

The behavior is that exec.Cmd.Wait() waits for process to close and MAY wait for passed stdio to close as well. Waiting on the stdio depends if it was backed by files or not.

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

No branches or pull requests

2 participants