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

Signal handling #27

Open
uazu opened this issue Mar 25, 2018 · 13 comments
Open

Signal handling #27

uazu opened this issue Mar 25, 2018 · 13 comments
Assignees
Labels
A-ergonomics Area: Ease of solving CLI related problems (e.g. filesystem interactions) C-tracking-issue Category: A tracking issue for an unstable feature

Comments

@uazu
Copy link

uazu commented Mar 25, 2018

The main ones are SIGINT (^C) and SIGTERM (kill). For CLI apps that require no special cleanup, the default handling is fine (i.e. exit immediately and let the OS cleanup).

For CLI apps that need to do special cleanup (e.g. flush to files or database, nicely terminate network connections, deconfigure devices, etc), these signal handlers need to be caught. Some possible models:

  • Catch signal, set a global flag, and have a call to check that global flag regularly in inner loops, and convert to an error to pass up the stack to do cleanup before exiting.

  • Try and do cleanup in the signal handler. But this is pretty hard as the calls that are permitted in a signal handler are quite restricted (see man 7 signal-safety on Linux)

In the case of catching ^C, if the global flag is already set, then on the second ^C consider exiting immediately, to give the user a way out if the CLI app is being unresponsive.

I don't know Windows. What's the model there for these kinds of signals?

If there's a crate already out there that handles this well, then someone link that here and perhaps we can consider this dealt with.

@kbknapp
Copy link
Contributor

kbknapp commented Mar 25, 2018

Thank you! I had meant to open an issue for exactly this yesterday and got side tracked.

For Windows I wonder if @retep998 would know?

@retep998
Copy link

Windows does not have signals. It does however have some similar mechanics.

  1. Console Console Handlers which handle several console events. You provide a callback and it calls your callback in a brand new thread whenever there is an event, where you can choose whether to let the default behavior occur. https://docs.microsoft.com/en-us/windows/console/console-control-handlers
  2. Structured Exception Handling which handles all the various types of system exceptions such as division by zero, invalid access exceptions, stack overflow, and so on.
  3. Window Messages which are how Windows provides most notifications to applications, but requires that the application have a window.

@tomwhoiscontrary
Copy link

A unix process can receive SIGWINCH when the user resizes the terminal it's running in. The right behaviour might be to do nothing, or to redraw some parts of the UI. It is definitely not to crash, which is something i have seen happen, so as a minimum, can we make sure that any signal handling we do doesn't overreact to SIGWINCH?

@tomwhoiscontrary
Copy link

Could termination signals be handled as a future? The application would obtain the future through some magical means; the signal handler would complete the future with the value of the signal; it would be up to the application to detect that it has completed and take some action.

@Screwtapello
Copy link

Screwtapello commented Jun 10, 2018

To summarize the linked SIGWINCH bug, as best I understand it:

  • The MySQL command-line client communicates with the server using straight-forward blocking I/O. It did not use SIGWINCH for anything, so it was left in its default configuration ("ignore").
  • Somebody submitted a patch to rewrap table output to fit the terminal. To handle the case where the terminal was resized after the application started, a SIGWINCH handler was installed.
  • Therefore if you resize the terminal while the client is waiting for a response from the server, the read() call is interrupted, and needs to be restarted.
  • However, the server I/O code was written under the assumption that the read() call would not be interrupted, and bails out in the face of this unexpected error.
  • The problem was fixed (accidentally?) by rewriting all the server I/O code. The ticket talks about a "framework" that was too big to back-port in a point release, although it doesn't mention what the framework did.

In the world of Rust, std::io::Read's provided methods read_to_end(), read_exact(), and (by reference) read_to_string() all say they automatically retry after interruption, so they're immune to this problem. But because it's a trait, the required method read() can't provide any such guarantees.

Unfortunately, I don't think there's anything a Rust CLI framework can do to prevent application code from overreacting to Read::read() returing ErrorKind::Interrupted, other than encourage people not to use it. For synchronous applications, I think using the provided methods (read_to_end(), etc.) should be enough for most use-cases; for asynchronous applications, hopefully Tokio/mio is smart enough to do the same thing.

@epage
Copy link
Contributor

epage commented Jul 1, 2018

Thoughts on signal-hook?

See also the blog post and reddit discussion

@yoshuawuyts
Copy link
Collaborator

Real world example of an application that's using custom signal handling. Figured it might be useful to gather examples.

@killercup killercup added this to the Preview 2 milestone Jul 18, 2018
@yoshuawuyts
Copy link
Collaborator

Also probably useful to enumerate some crates for signal handling. If anyone has seen any others, it'd be great to know!


I feel there are a good amount of solutions out there already. It might be useful to try and enumerate which parts of these libraries are found lacking, and possibly contribute!

I'm personally really interested in figuring out ways to make documentation of signal handlers. A thing that might help a lot would probably be if there's some consistency in how signals were defined. I'm not certain what the right approach for this might be, but that might be worth exploring further!

Wrapping Up

I hope all this makes sense; I'm real keen to hear what other folks think of this!

@killercup
Copy link
Collaborator

Two aspects here are what crates to recommend when people look into this, and how to document which signals an app supports.

I'll assign this to me but will ping people who commented here to find consensus around what to do next on this issues.

@killercup killercup self-assigned this Jul 24, 2018
@epage epage mentioned this issue Jul 24, 2018
@tvannahl tvannahl mentioned this issue Jul 27, 2018
2 tasks
@killercup killercup removed this from the Preview 2 milestone Jul 31, 2018
@yoshuawuyts
Copy link
Collaborator

chan-signal was deprecated in favor of signal-hook today — https://users.rust-lang.org/t/ann-chan-is-deprecated-use-crossbeam-channel-instead/19251

@killercup
Copy link
Collaborator

Hey, I'm about to write the signal chapter for the book, based on the info in this issue. Thanks again for the great summary, @uazu!

The ctrlc crate seems to be a good start for getting into handling SIGINT and the equivalent on Windows, so I want to start with recommanding that. I've not seen much Rust-on-Windows specific. Is there any other common pattern/crate I should mention, @retep007?

On top of that I want to mention signal-hook for UNIX platforms.

@ghost
Copy link

ghost commented Dec 4, 2018

@killercup Related issue: rust-lang-nursery/rust-cookbook#501

If you need some inspiration, here's a simple stopwatch that gets stopped on Ctrl-C (and I love how it combines signal-hook and crossbeam-channel with select!):
https://github.com/crossbeam-rs/crossbeam/blob/master/crossbeam-channel/examples/stopwatch.rs

@killercup
Copy link
Collaborator

Oh, very interesting! Thanks! I'll probably need to make this more CLI-related but I might end up stealing that example as it's the next logical step to use this with crossbeam-channel :)

killercup added a commit to killercup/cli-wg that referenced this issue Dec 7, 2018
@epage epage added C-tracking-issue Category: A tracking issue for an unstable feature A-ergonomics Area: Ease of solving CLI related problems (e.g. filesystem interactions) labels Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ergonomics Area: Ease of solving CLI related problems (e.g. filesystem interactions) C-tracking-issue Category: A tracking issue for an unstable feature
Projects
None yet
Development

No branches or pull requests

8 participants