-
Notifications
You must be signed in to change notification settings - Fork 92
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
Cmd pdeathsig #943
Cmd pdeathsig #943
Conversation
In almost all children we fork, we want the child to reliably exit if we do (e.g. especially if we panic). The Linux PR_SET_PDEATHSIG is just great for this. Signed-off-by: Colin Walters <walters@verbum.org>
I just happened to glance at this code, this gives us stderr in the error, etc. Signed-off-by: Colin Walters <walters@verbum.org>
fa9a992
to
479cbca
Compare
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.
This could go in as-is, but see comments/questions below (mostly superficial, or for the sake of my own curiosity)...
@@ -84,6 +88,19 @@ impl CommandRunExt for Command { | |||
self.status()?.check_status(stderr) | |||
} | |||
|
|||
#[allow(unsafe_code)] | |||
fn lifecycle_bind(&mut self) -> &mut Self { | |||
// SAFETY: This API is safe to call in a forked child. |
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.
A docs reference would be good (even if it's just to say that this is implemented with prctl and point to the list in signal-safety manpage)...
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.
Hmm, the rustix docs for this function do link directly to the prctl man page right?
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 while we could have more safety explanations in cases like this, what we have here is actually better than some of the other (few) unsafe
cases and if we were to try to raise the bar we'd need to a bit more
utils: Add a lifecycle_bind helper for Command
In almost all children we fork, we want the child to reliably
exit if we do (e.g. especially if we panic). The Linux
PR_SET_PDEATHSIG is just great for this.
Signed-off-by: Colin Walters walters@verbum.org
utils: Use
run
helper for editorI just happened to glance at this code, this gives us stderr
in the error, etc.
Signed-off-by: Colin Walters walters@verbum.org