smol-rs / async-signal

Asynchronous signal handling
Apache License 2.0
14 stars 6 forks source link

Stabilize this crate #31

Open notgull opened 4 months ago

notgull commented 4 months ago

I realized v0.2.0 of this crate a little over a year ago. Since then it has not had any breaking public API changes. It's used in production via async-process and we've only ever gotten one bug report with it (the now-removed signalfd backend).

So, I think this crate is stable and can be released as version 1.0.0. Are there any API changes I should make before releasing the stable version of this crate publicly?

cc @smol-rs/admins

taiki-e commented 4 months ago

AFAIK, there are very few direct users of this crate at this time. https://crates.io/crates/async-signal/reverse_dependencies So I suspect that few people are familiar enough with this crate to have opinions on the API of this crate in the first place.

Also, I have heard before that the Windows implementation is broken (https://github.com/smol-rs/async-signal/pull/20#discussion_r1342146572). What is the current status of that? It is also interesting that tokio::signal provides different APIs for Unix and Windows.

notgull commented 3 months ago

So I suspect that few people are familiar enough with this crate to have opinions on the API of this crate in the first place.

This is probably right.

Also, I have heard before that the Windows implementation is broken (#20 (comment)). What is the current status of that?

Needs more testing, I need to get my hands on Windows hardware.

It is also interesting that tokio::signal provides different APIs for Unix and Windows.

Yeah, the only common "signal" is the Ctrl-C signal, which both platforms expose. In async-signal I only allow for this signal to be used on Windows.

fogti commented 3 months ago

Huh, I though some additional signals were also available on windows (e.g. SIGFPE): https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/signal

notgull commented 3 months ago

The other signals are either not raised in practice or impractical to handle from an async perspective. See here: https://github.com/smol-rs/async-signal/blob/master/src/windows_registry.rs

fogti commented 3 months ago

"This function is safe; it is only unsafe for consistency."

consistency with what?

on the other hand, wouldn't is make sense to handle more/all possible events which could be handled with SetConsoleCtrlHandler?

notgull commented 2 months ago

consistency with what?

In order to be consistent with signal_hook_registry::register.

on the other hand, wouldn't is make sense to handle more/all possible events which could be handled with SetConsoleCtrlHandler?

I want to keep this crate scoped to signals.