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

Making signal API real-time signal aware #2451

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

ChrysoliteAzalea
Copy link

Currently, the nix::sys::signal::sigaction() function uses the Signal enum, which only covers standard signals, but not real-time signals. Using real-time signals with nix requires using the std::mem::transmute() function to assign an impossible value to the Signal enum, which is undefined behavior (and is unsafe for any other use than passing to nix functions, and possible even then).

What does this PR do

In this PR, the nix::sys::signal API is made real-time signal aware by adding the new enum SignalValue that has two variants: Standard that takes Signal values, and Realtime, that takes real-time signal numbers SIGRTMIN+n, where n is the passed number. Most of the functions are left untouched, instead, real-time signal aware versions of the were added with the rt_ prefix (similar to the real-time signal aware system calls in the Linux kernel that glibc uses instead of the old system calls). However, two significant changes have been made: turning SigSet into iterator returns a SignalSetIter instead of real-time signal unaware SigSetIter, and the signal field of structs SigevSignal and SigevThreadId takes the new enum instead of the old one.

Checklist:

  • [yes] I have read CONTRIBUTING.md
  • [yes] I have written necessary tests and rustdoc comments
  • [yes] A change log has been added if this PR modifies nix's API

@ChrysoliteAzalea
Copy link
Author

I've managed to work out most issues with tests, but I don't know why these three are failing, and why they are failing on mips and pass on everything else. I think I'll have to set up a VM emulating a mips CPU to solve it.

@SteveLauC
Copy link
Member

Hi, thanks for the PR!

I will take a deeper look at how to add realtime signal support and this PR this weekend

@ChrysoliteAzalea
Copy link
Author

ChrysoliteAzalea commented Jul 22, 2024

I've run into a problem while setting up the MIPS virtual machine -- almost none of the mainstream Linux distrubutions support MIPS anymore. Also, the targets for which these tests are failing are not available in rustup target list, so I wonder, is it worth keeping them?

P.S. If you can advise where a MIPS virtual machine images can be downloaded (with Rust compiler), I can try to work these issues out.

@SteveLauC
Copy link
Member

Also, the targets for which these tests are failing are not available in rustup target list

This is because they are in tier 3.

so I wonder, is it worth keeping them?

In Nix's platform support, they are in tier 1, so we cannot drop them.

QEMU can have problems, so if there are really weird, just ignore the tests under QEMU #[cfg_attr(qemu, ignore)], but we should document the reason.

About this PR, generally it looks good, I just haven't made a thought about the interface design as this will be a breaking change. I want to make a thorough thought before taking any further actions, sorry about this.

@ChrysoliteAzalea
Copy link
Author

In Nix's platform support, they are in tier 1, so we cannot drop them.

I'd like to ask, where can I download the QEMU virtual machine image for MIPS with Rust to work these issues out?

@SteveLauC
Copy link
Member

In Nix's platform support, they are in tier 1, so we cannot drop them.

I'd like to ask, where can I download the QEMU virtual machine image for MIPS with Rust to work these issues out?

Try cross? That's what the CI is using.

@ChrysoliteAzalea
Copy link
Author

Thank you! I've installed a test environment using cross and successfully reproduced an error. I'll try to solve this issue.

@ChrysoliteAzalea
Copy link
Author

I've managed to figure out the exact problem with the mips tests:

  • glibc reserves two real-time signals for inter-thread communication (one for timers and thread cancellation, one for system calls like setuid() to be invoked from all threads when one thread invokes them), as described in man 7 nptl. These signals should not be used by application (because it would break NPTL logic and can break application logic, because glibc may send these signals to threads), and glibc hides these signals (for example, by altering the value of SIGRTMIN). SignalValueIterator may reach these values during iteration. Previously, if it reached the value that is not a valid standard signal, it assumed that it's always a reserved real-time signal and therefore skipped to SIGRTMIN as defined by glibc
  • As it turned out, SIGEMT is not defined for mips in this crate, therefore, it is not considered a valid standard signal. When SignalValueIterator reached this signal (it had the value 7), it leaped to SIGRTMIN, skipping all the other signals between SIGEMT and SIGRTMIN, including SIGUSR1 and SIGUSR2 used in tests. Last commits implement graceful handling of missing standard signals by seeking a nearest valid standard signal.

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

Successfully merging this pull request may close these issues.

2 participants