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 revert_to_default() or similar #106

Open
staticfloat opened this issue Jun 27, 2023 · 6 comments
Open

Add revert_to_default() or similar #106

staticfloat opened this issue Jun 27, 2023 · 6 comments

Comments

@staticfloat
Copy link

I see that in 3.1.2 an example was added to demonstrate how killing the process from a CTRL-C handler can be done, however this masks an important distinction, which is that the process that launched our process can tell the difference between exit(0) and termination due to raise(SIGINT), through WTERMSIG() and friends.

Rust itself provides a small snippet to show how to reset the SIGINT signal handler to SIG_DFL, which would then allow a raise(SIGINT) to terminate the rust process in the same way as if ctrlc was not used.

This is particularly useful for "wrapper" executables such as juliaup where it attempts to as transparently as possible launch a child process, and exit in the same way the child process exits. In this scenario, if the child process dies due to a SIGINT, we want the wrapper process to also look like it died due to a SIGINT.

@Detegr
Copy link
Owner

Detegr commented Jun 29, 2023

I'd like to see this feature implemented in a way where the user would set the handler with a builder of some sort that had an option to not inherit the signal handlers for a child processes. However in practice I think this is very tricky, if not impossible.

Could you have a wrapper executable in juliaup that would (re)set the handlers to SIG_DFL using libc or nix crates and then launch the child process?

@staticfloat
Copy link
Author

Could you have a wrapper executable in juliaup that would (re)set the handlers to SIG_DFL using libc or nix crates and then launch the child process?

Yes, juliaup is that wrapper executable. We can of course manually use libc crates to undo what ctrlc has done, but I believe that if ctrlc contains functionality to set default signal handling for a signal, it makes sense for ctrlc to also provide functionality to reset signal handling back to the default, otherwise if ctrlc changes something about how it performs its signal overloading, we may need to adjust our resetting in the future. I'd rather not have code in the higher context (juliaup) that is so intimately coupled with the implementation details of ctrlc.

@Detegr
Copy link
Owner

Detegr commented Jul 18, 2023

If I understand correctly, in juliaup you would set signal handlers back to default, launch the child process and then set the signal handlers back, am I right? The issue I see with that approach is that it would result in code where a SIGINT could kill the program if it hits the part of the code that had the signal handlers reverted back to default. To safely handle this I think you'd need to launch a wrapper subprocess first that reverts its signal handlers and then launches the actual subprocess, that way the main juliaup binary would always be able to handle its signals correctly.

Now that I wrote it out, I realized it may still be beneficial to have the feature as you could use it in the wrapper child process to revert the handlers, should you decide to implement it that way.

@staticfloat
Copy link
Author

If I understand correctly, in juliaup you would set signal handlers back to default, launch the child process and then set the signal handlers back, am I right?

Not quite, we start up the signal handlers as soon as the child process launches (ignoring CTRL-C in the launcher so that the child process can receive it correctly), but then after the child process exits, we want to set the signal handlers back to default. This is because if the child process gets killed due to SIGINT, we want to be able to then kill ourselves with a SIGINT and have someone who is waiting on the wrapper process to be able to determine that this is why the underlying process exited. Specifically, we raise() the signal that the child process died due to, but this raise() is ineffective without having some way to undo what the ctrlc crate does.

@samuela
Copy link

samuela commented Jan 11, 2024

I'm also looking for the ability to remove a ctrlc handler. In my case I'm interested in building a CLI interaction library (think readline) that ought to handle ctrl-c internally, but then should return things to their original state when handing back control to the user.

It seems logical to me that ctrlc::set_handler return a "handle object" that, when dropped, removes the signal handler.

@Detegr
Copy link
Owner

Detegr commented Mar 9, 2024

The handle object approach would not work with child processes though as they inherit the signal handlers (on *nix). Now that I'm reading this discussion again I'm starting to think a separate function for setting signal handlers back to SIG_DFL would actually be nice and make sense. Being a separate function it would not break any existing code.

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

3 participants