vorner / signal-hook

Rust library allowing to register multiple handlers for the same signal
Apache License 2.0
705 stars 71 forks source link

Use rustix instead of libc #142

Closed notgull closed 1 year ago

notgull commented 1 year ago

rustix is a Rust-idiomatic interface to various system calls. Aside from being more idiomatic than normal system calls, it's claim to fame is that it bypasses libc to use direct system calls on Linux. In some cases, the system calls can be directly inlined into their calling functions.

rustix recently merged support for signal handling. It would be nice if this crate used those functions instead of libc's, in order to optimize these functions.

vorner commented 1 year ago

Hello

Does it also work on all the other platforms signal-hook supports? What is its minimal rust version? Does it also provide functions for all the rest?

notgull commented 1 year ago

Does it also work on all the other platforms signal-hook supports?

It doesn't work on Windows, but we could just keep using libc there... or, preferably, the actually underlying Win32 calls, but that's another issue.

What is its minimal rust version?

It's minimal Rust version is tied to Debian Stable; as of the time of writing, it's 1.48. This should be low enough for most realistic use cases.

Does it also provide functions for all the rest?

For now, it provides all of the functions needed to reimplement signal-hook-registry. I'm not sure if it provides things like raise for signal-hook, but we could just implement it in the registry crate first.

vorner commented 1 year ago

Well, from signal-hook-registry's docs:

Currently builds on 1.26.0 an newer and this is very unlikely to change. However, tests require dependencies that don’t build there, so tests need newer Rust version (they are run on stable).

It's in a 1. release… I'd say 1.26 is somewhat ancient, but I'd still be a bit reluctant to change that requirement if there isn't a good motivation.

Considering there would be a „split“ between implementation on windows, I'm not sure how other platforms (android?, solaris?) are supported and rustix is 0. crate currently (at least according to docs.rs) ‒ which is problematic as a dependency of „stable“ package, I'd say I don't see the compelling reason to go to all that trouble.

As I see it, I don't mind someone experimenting with that and watching it evolve, but I'd prefer to be conservative about switching it in a „real“ release until rustix stabilizes more.

notgull commented 1 year ago

Sounds good (and, from discussion on Rustix, it may be a bad idea to do signal handling outside of the libc anyways).

However, it's my understanding that, in the Rust community, bumping the minimum supported Rust version corresponds with a minor version bump rather than a major one. I forget what the exact reasoning for this is, but a bump in MSRV doesn't also mean a bump in the major version.

In addition, I doubt that anyone is using rustc 1.26, a version of Rust that is almost five years old at this point. Even Debian Stable, famed for using old versions is software, uses rustc 1.48.

notgull commented 1 year ago

After discussion with the rustix maintainers, this is probably a bad idea. signal handling should be done through the libc