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

wait: implement waitid() #1584

Merged
merged 1 commit into from
Mar 10, 2022
Merged

wait: implement waitid() #1584

merged 1 commit into from
Mar 10, 2022

Conversation

neocturne
Copy link
Contributor

@neocturne neocturne commented Oct 23, 2021

waitid() has a number of additional features that waitpid() is missing:

  • WNOWAIT is only accepted for waitid() on Linux (and possibly other platforms)
  • Support for waiting on PID file descriptors on Linux

For now support is added for all platforms with waitid() that have proper siginfo_t support in libc. NetBSD support is currently a work in progress [1].

Tests for the signal/exit code are currently skipped on MIPS platforms due to multiple bugs in qemu-user in the translation of siginfo_t (one fixed in January [2], one currently under review [3]).

[1] rust-lang/libc#2476
[2] https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg04810.html
[3] https://lists.nongnu.org/archive/html/qemu-devel/2021-10/msg05433.html

@neocturne neocturne marked this pull request as draft October 23, 2021 11:38
@neocturne neocturne force-pushed the waitid branch 4 times, most recently from 293f405 to 095b08a Compare October 23, 2021 20:32
@neocturne neocturne marked this pull request as ready for review October 23, 2021 20:40
@neocturne neocturne force-pushed the waitid branch 2 times, most recently from 197652a to ad72c47 Compare October 24, 2021 10:00
@asomers asomers added this to the 0.24.0 milestone Nov 16, 2021
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget a CHANGELOG entry too

/// Wait for a process to change status
///
/// See also [waitid(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/waitid.html)
#[cfg(any(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libc defines this function for almost all OSes. I think it might be everything except Redox. Any reason not to enable it for all of them in Nix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started with the list of platforms for which WEXITED is defined in WaitPidFlag and then removed all that had some obvious issue:

  • NetBSD has broken siginfo_t in libc (see issue linked in PR description)
  • Apple/IOS are missing CLD_EXITED/KILLED/DUMPED in libc (and I don't really want to open another libc PR for a system that I do not use and that I'm not familiar with...)

src/sys/wait.rs Outdated Show resolved Hide resolved
@rtzoeller
Copy link
Collaborator

@NeoRaider can you rebase this? It isn't conflicting on GitHub, but local testing shows the changelog will merge into the 0.23.1 patch's section, rather than the current one.

@neocturne
Copy link
Contributor Author

Rebased and updated commit message to reflect the current status of the mentioned qemu-user issue.

@neocturne
Copy link
Contributor Author

Hmm, seems I'll have to update the test code as well.

@neocturne neocturne force-pushed the waitid branch 2 times, most recently from 5c83b03 to 80de838 Compare March 6, 2022 00:43
@rtzoeller
Copy link
Collaborator

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 9, 2022

Merge conflict.

@rtzoeller
Copy link
Collaborator

Another PR went in first and modified the changelog in the same spot. Can you rebase?

waitid() has a number of additional features that waitpid() is missing:

- WNOWAIT is only accepted for waitid() on Linux (and possibly other
  platforms)
- Support for waiting on PID file descriptors on Linux

For now support is added for all platforms with waitid() that have proper
siginfo_t support in libc. NetBSD support is currently a work in progress
[1].

Tests for the signal/exit code are currently skipped on MIPS platforms due
to bugs in qemu-user's translation of siginfo_t (fixed in [2] and [3]; the
second fix is not in a released qemu version yet).

[1] rust-lang/libc#2476
[2] https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg04810.html
[3] https://lists.nongnu.org/archive/html/qemu-devel/2021-10/msg05433.html
@rtzoeller
Copy link
Collaborator

bors r+

@bors bors bot merged commit 0435be2 into nix-rust:master Mar 10, 2022
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.

None yet

3 participants