-
Notifications
You must be signed in to change notification settings - Fork 354
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
Address ECHILD #1777
Address ECHILD #1777
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1777 +/- ##
==========================================
- Coverage 68.67% 68.64% -0.04%
==========================================
Files 121 121
Lines 13318 13324 +6
==========================================
Hits 9146 9146
- Misses 4172 4178 +6 |
Signed-off-by: utam0k <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. The only thing we need to check is that there won't be a race (where libcontainer succeeds in reaping the process before the other waitpid user does it). But that's not really youki's concern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
_ => (), | ||
Ok(_) => (), | ||
Err(err) => { | ||
if err == Errno::ECHILD { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Can this be turned into Err(Errno::ECHILD)
case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need to do it because waitpid
returns Result<T, Errno>
https://docs.rs/nix/latest/nix/type.Result.html
Signed-off-by: utam0k <[email protected]>
Assume the case where intermediate_process is waitpid by someone and ignore ECHILD in that case.
This is safe. This is safe because intermediate_process and main_process check if the process is finished by piping instead of exit code.