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

Stabilize this crate #31

Open
notgull opened this issue Apr 20, 2024 · 6 comments
Open

Stabilize this crate #31

notgull opened this issue Apr 20, 2024 · 6 comments

Comments

@notgull
Copy link
Member

notgull commented Apr 20, 2024

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
Copy link
Collaborator

taiki-e commented Apr 21, 2024

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 (#20 (comment)). What is the current status of that?
It is also interesting that tokio::signal provides different APIs for Unix and Windows.

@notgull
Copy link
Member Author

notgull commented May 27, 2024

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
Copy link
Member

fogti commented May 28, 2024

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

  • SIGABRT Abnormal termination
  • SIGFPE Floating-point error
  • SIGILL Illegal instruction (ANSI compat)
  • SIGINT CTRL+C signal
  • SIGSEGV Illegal storage access
  • SIGTERM Termination request (ANSI compat)

@notgull
Copy link
Member Author

notgull commented May 28, 2024

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
Copy link
Member

fogti commented May 28, 2024

"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
Copy link
Member Author

notgull commented Jun 2, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants