Migrate Unix implementation to signal-hook-registry

[notgull]

Closes #97 by migrating the Unix backend to signal-hook-registry.

To avoid a breaking change the signal-hook-registry errors are translated to EINVAL, since that's what they usually are anyways.

[notgull]

@Detegr Poke, any chance you can merge this? CI is failing because of spurious networks failures in AppVeyor.

[Detegr]

I still don't know how I feel about this. signal-hook-registry seems like a solid library and it's in use by large crates like tokio, so it's not that I don't trust they're doing things the right way.

But I feel the crate does too much and using it would defeat the purpose of this crate. ctrlc would essentially be just a wrapper to the crate with some separate Windows-specific code sprinkled in instead of being a separate implementation for doing signal handling. If I were to merge this, and accept that this crate essentially turns into a wrapper crate for signal-hook-registry, I'd like to handle the Windows side with it as well. signal-hook-registry seems to have some Windows support as well but it does not seem to use windows-sys so maybe it works only with MinGW or the likes?

I get that it's not ideal that having the both crates in a dependency chain just breaks things, that's something I'd like to fix and proposed a fix for that earlier. However, I'd like to see a concrete example of such dependency chain. From a brief look, uses of signal-hook in a library crates (instead of applications) are quite few, usually handling SIGWINCH for terminal resizes, not SIGINT or SIGTERM. I'd argue having built-in signal handling in a library crate is not something that should be done. Maybe there are exceptions, but I'm not aware of such exceptions.

[notgull]

But I feel the crate does too much and using it would defeat the purpose of this crate. ctrlc would essentially be just a wrapper to the crate with some separate Windows-specific code sprinkled in instead of being a separate implementation for doing signal handling.

This is probably the best way of doing it, as this is what async-signal does.

If I were to merge this, and accept that this crate essentially turns into a wrapper crate for signal-hook-registry, I'd like to handle the Windows side with it as well. signal-hook-registry seems to have some Windows support as well but it does not seem to use windows-sys so maybe it works only with MinGW or the likes?

Signals in Windows CRT are practically just a wrapper around console control handlers, so it doesn't have to be this way.

I get that it's not ideal that having the both crates in a dependency chain just breaks things, that's something I'd like to fix and proposed a fix for that earlier.

Ideally, the best way of doing this is to avoid stepping on others' toes is to just use this strategy.

However, I'd like to see a concrete example of such dependency chain. From a brief look, uses of signal-hook in a library crates (instead of applications) are quite few, usually handling SIGWINCH for terminal resizes, not SIGINT or SIGTERM. I'd argue having built-in signal handling in a library crate is not something that should be done. Maybe there are exceptions, but I'm not aware of such exceptions.

It's less of "this does break something right now" and more of "this could cause things to break in an extremely hard-to-debug and potentially unsound way in the future". I'm not aware of any cases where ctrlc and signal hook are used in the same dependency graph, but if it does happen you could end up with a very messy situation.

[bredbord]

I'm with @notgull on this one. In my opinion there's little harm in following the structure of a largely accepted, solid implementation like async-signal; there's no need to reinvent the wheel from a logistics standpoint. Arguably, it's better and safer to have some miscellaneous features (being the windows-specific code) instead of risking the possibility of instability in the hypothetical dependency chain.

[Detegr]

I decided against using a multi-hundred line dependency for fixing a hyphotetical bug, and implemented a simple "see if we have signal handler installed, and error out if we do" strategy instead.

If signal-hook is used to register a handler before this crate, the set_handler function will now fail. If this crate is used before signal-hook, both of the handlers are called. AFAIK that's how it would be if this crate was using signal-hook internally.

[Detegr]

This change is breaking a lot of other people's stuff and I'm regretting implementing it.

I moved the checking implementation to ctrlc::try_set_handler and changed ctrlc::set_handler back to the previous functionality. Sorry for the inconvenience caused by this change.

[bredbord]

Good deal. Seems like that should do the trick for now. Too bad there isn't another, robust implementation hanging around.