Skip to content

Conversation

@ids1024
Copy link
Contributor

@ids1024 ids1024 commented Jun 11, 2025

Supported on Linux since 6.5.

I see socket_peercred is only provided on Linux, while OpenBSD (https://man.openbsd.org/getsockopt.2) has some version of SO_PEERCRED, and FreeBSD (https://man.freebsd.org/cgi/man.cgi?unix(4)) has a LOCAL_PEERCRED (libwayland for instance has code to use those on their respective platforms). These APIs do seem to be slightly different though. In any case, SO_PEERPIDFD is definitely Linux-specific for the time being.

@ids1024
Copy link
Contributor Author

ids1024 commented Jun 11, 2025

pidfd_getpid would also be good to add. Apparently that's not a system call, so for linux-raw we'd need to parse /proc/self/fdinfo/$pidfd for the Pid: field. But that should be fine, and not too hard.

Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

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

This looks good to me, with one quick question:

#[test]
#[serial] // for `setrlimit` usage, and `crate::init`
fn test_select_with_maxfd_sockets() {
use rustix::fd::{FromRawFd as _, OwnedFd};
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious why these lines were needed in the tests, that weren't otherwise changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That fixes a compile error with cargo test --features net,process,event. Though it also works to use --features=all-apis like CI uses. (I suppose ideally it shouldn't error for any combination of features; but doesn't matter that much for tests.)

@sunfishcode
Copy link
Member

Those imports are unfortunately evoking warnings, which are currently configured to be errors in CI. Could you add conditionals to the imports so that they're not imported if they're not needed?

@ids1024
Copy link
Contributor Author

ids1024 commented Sep 12, 2025

Huh, looking at CI, I guess libc doesn't define SO_PEERPIDFD in https://github.com/rust-lang/libc/blob/main/src/unix/linux_like/android/mod.rs.

Not sure why those are defined separately for Android; I'd assume unless the C standard library is doing something weird, those particular definitions should be the same for anything with a Linux kernel.

@rusty-snake rusty-snake mentioned this pull request Oct 16, 2025
8 tasks
@zeenix
Copy link

zeenix commented Oct 16, 2025

@ids1024 Thanks so much for doing this. Do you intend to finish this PR?

@ids1024 ids1024 force-pushed the socket_peerpidfd branch 3 times, most recently from e7427e1 to 34551eb Compare October 16, 2025 19:00
@ids1024
Copy link
Contributor Author

ids1024 commented Oct 16, 2025

I guess to pass CI this could just be disabled on Android. Presumably libc should be updated to have things like SO_PEERPIDFD. Changed that for now.

Tests seem to be broken with a recent compile, so I rebased this on #1523. Looks like there are a couple other unrelated CI errors? But I think this should be mergeable.

@ids1024 ids1024 force-pushed the socket_peerpidfd branch 2 times, most recently from e8980ab to 9f77ef6 Compare October 16, 2025 19:34
@ids1024
Copy link
Contributor Author

ids1024 commented Oct 16, 2025

Or well, one of the errors was related to this change. Fixed that, and added a commit for another CI error. So now everything is passing.

@ids1024 ids1024 requested a review from sunfishcode October 16, 2025 19:55
@ids1024 ids1024 force-pushed the socket_peerpidfd branch 2 times, most recently from db25968 to e749200 Compare November 4, 2025 17:09
@sunfishcode
Copy link
Member

I haven't investigated this yet myself, but CI is failing on a missing SO_PEERPIDFD on nightly.

Supported on Linux since 6.5.
@ids1024
Copy link
Contributor Author

ids1024 commented Nov 14, 2025

I guess the last time I rebased this I did it on a different computer without pulling the latest version of the branch; so it lacked a fix to not build enable the feature on Android.

Presumably Android should support this too, but that will require a PR to libc to add the necessary constants.

This should be mergeable now.

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.

3 participants