Use signal-hook-registry for listening to signals

[notgull]

It is unsound to use this crate and signal-hook in the same dependency graph, since they will both try to acquire the signal handler and one will overwrite the other. This issue can be fixed by changing the signal handling backend to rely on the signal-hook-registry crate.

[Detegr]

Hi, thank you for your contribution.

What is the use case where there would be multiple crates handing signals? This crate and signal-hook are more or less doing the same thing internally, so it makes sense that they conflict.

Trying to be compatible with other crates doing signal handling feels a bit dangerous and error prone to me. I would rather "fix" this by implementing this TODO so that registering signals would fail if there's already a handler in place.

[notgull]

What is the use case where there would be multiple crates handing signals?

There could be multiple crates in the dependency graph where one could use signal-hook and another could use ctrlc.

Trying to be compatible with other crates doing signal handling feels a bit dangerous and error prone to me.

On the contrary, it's actually less error prone to just use one universal crate for signal handling.

I would rather "fix" this by implementing this TODO so that registering signals would fail if there's already a handler in place

This would be an acceptable solution, since signal-hook-registry handles the case where there is already a solution. However, I think it would be better to just be signal-hook-registry compatible.

[Detegr]

Fixed with release 3.3.0.

[jdx]

this change makes my CLI break when I do the following in a bash script:

trap -- '' SIGINT;
mycli

The error:

The application panicked (crashed).
Message:  Error setting Ctrl-C handler: MultipleHandlers
Location: src/main.rs:93

is this intended? or was this change only suppose to impact when a signal was caught within the same process? I suppose I can simply ignore the error but wanted to make sure this is working as expected.

[Detegr]

I think in that case the mycli program inherits the signal handlers from the parent, so it is working as expected in that regard. The previous implementation would've just overwritten the handler. mycli could set the handler manually to SIG_DFL beforehand if the application wanted to have its own signal handling.