0.37 release planning

[sunfishcode]

With #487 and #486 landed, we have an upcoming need for a semver bump to 0.37. Please post any questions or concerns here.

If they're fixed soon, it'd be nice to pull on the changes for #495 and/or #493 here too.

[notgull]

I was hoping that I could get #488 into shape as well, as well as implement a passable API for Solarish's event ports API so that this crate can be used in polling for all of the *nix backends.

Also, if we're doing breaking changes, is there a way that we can adjust dup2 and dup3's APIs? Having to take an OwnedFd as a parameter hurts them, especially in the case of dup3, where having an option for CLOEXEC is beneficial.

[SUPERCILEX]

Regarding dup, Alan noticed this too: nix-rust/nix#1863 (comment). I couldn't figure out a good way to both require ownership and not force you to pass in the OwnedFd, so I ended up adding

fn dup2_raw<Fd1: AsFd, Fd2: AsRawFd + IntoRawFd>(oldfd: Fd1, newfd: Fd2) -> Result<OwnedFd> {
    let res = unsafe { libc::dup2(oldfd.as_fd().as_raw_fd(), newfd.as_raw_fd()) };
    Errno::result(res).map(|_| unsafe { OwnedFd::from_raw_fd(newfd.into_raw_fd()) })
}

on top of the existing dup2 (and same for dup3). That kind of sucks, but it lets us have two safe interfaces that can be used at different levels of abstraction. Would be cool if there was a better way though!

---

Regarding #493, I just need a final sign-off and I'll send a PR.

[sunfishcode]

By I/O safety, that dup2_raw should be unsafe, because it operates on raw fds (AsRawFd/IntoRawFd are not trustable).

[SUPERCILEX]

Even though it will handle closing the FD for you if it's owned? @ids1024 argued the same position, so I'm probably in the minority and would be fine with it being unsafe. I don't see a way to let you create create FDs out of thin air without raw FDs (well you could indirectly with BorrowedFd, but that would be icky), so that's why I landed on that solution.

[sunfishcode]

RawFd implements AsRawFd and IntoRawFd and it's just an i32 so you can pass in 42 and have it clobbered without knowing who owns 42.

Rustix's answer here is that you can OwnedFd::from_raw_fd if you really want to dup2 to an arbitrary integer.

[sunfishcode]

Rust code is allowed to assume that the identity of a file descriptor does not change without someone having access to the &mut OwnedFd. This is important for safety in several use cases, like mmapping a memfd you previously sealed.

[SUPERCILEX]

Rustix's answer here is that you can OwnedFd::from_raw_fd if you really want to dup2 to an arbitrary integer.

I don't think that's a good solution in the face of errors since you'll need to handle ManuallyDrop and friends yourself.

Rust code is allowed to assume that the identity of a file descriptor does not change without someone having access to the &mut OwnedFd.

This is a compelling argument for dup2_raw being unsafe, I agree now, thx. :)

[ids1024]

Yeah, OwnedFd::from_raw_fd for an fd that isn't open isn't quite right. You want to avoid dropping it, and technically just constructing an OwnedFd from an invalid fd may be considered unsound, though it's unlikely to be implemented in a way that could cause an issues.

If this is needed, having a variant of the function that's unsafe and takes a RawFd is probably best.

[sunfishcode]

If you have a RawFd which is invalid, meaning it's forged and potentially aliasing an OwnedFd somewhere else in the process, then doing a dup2_raw on it is unsound, in general. Constructing a new OwnedFd isn't the problem.

It gets ambiguous in the case where you're doing dup2's in preparation for an exec, and you know all the fds you want to pass to the exec, and you want to just overwrite any other fds that might be in the way. Ordinarily that should be considered unsound. At the same time, this is the main use case for dup2. So we need some rule to say it's ok in this specific case, and we don't yet have it.

[sunfishcode]

I filed #497 to discuss I/O safety for this dup2 issue.

[ids1024]

Constructing a new OwnedFd isn't the problem.

It shouldn't be unsound as implemented, or probably in any reasonable future implementation of std, but I'm just noting that you technically violate what OwnedFd::from_raw_fd says is required for safety:

Safety
The fd passed in must be a valid and open file descriptor.

Safety
The resource pointed to by fd must be open and suitable for assuming ownership. The resource must not require any cleanup other than close.

And it just isn't quite right semantically, since an fd that isn't open isn't what an OwnedFd is supposed to represent.

[sunfishcode]

And it just isn't quite right semantically, since an fd that isn't open isn't what an OwnedFd is supposed to represent.

It isn't known to be sound to pass in what you believe is a fd that isn't open to dup2_raw, because in general you don't know what file descriptors are open elsewhere in the process. So the OwnedFd::from_raw_fd requirement reflects what is currently known to be sound.

That doesn't cover everything that people expect to be able to do with dup2_raw, but that's where #497 comes in.

[SUPERCILEX]

On a different but breaking-ish topic, are we sure we want statx to to check for its own availability? I'm not a big fan of that because it bumps up codegen for everyone even if they don't care about compatibility with 4.x kernels. The rationale for this check is that the stdlib does this, so rustix should too. I don't believe this transitivity holds. The stdlib does this because it must be able to stat and would like to do so more efficiently when possible. On the other hand, Rustix offers all the stat functions, so developers choose which ones they want.

I think if statx gets special treatment, then a minimum kernel version should be chosen and all syscalls (such as io uring) should be using this pattern. But again, I don't think that's a good idea since everyone will have different min kernel versions and it'll be a perf hit. If people really want this, I think the better approach would be to write a wrapper library that does this.

Thoughts?

[sunfishcode]

Yeah, I'm open to making changes here.

Creating a whole extra library just to do "don't call statx again if it's just going to ENOSYS", a common need, is inconvenient. Would it work for you to rename the existing statx to statx_if_avail or so, and then add a new regular statx which doesn't do the extra code?

(io_uring is a different situation because if you don't have io_uring, you typically take a completely different path and never try to call it again)

[SUPERCILEX]

io_uring is a different situation because if you don't have io_uring, you typically take a completely different path and never try to call it again

Fair point, but what about openat2 or copy_file_range?

And yeah, I agree that a sperate lib would be a pain, but duplicating every function in the public API also seems unpleasant. Maybe we can use features for this? You could say "assume kernel x.x or later". Or maybe we can provide a macro keyed by the name of the syscall that wraps an arbitrary function returning the rustix result?

I think I'd vote for either the feature gate idea or yours (though I worry about the API complexity).

[notgull]

I'd be okay with an "assume kernel version 4.XX" feature that elides this check. Although, what happens when Rust's minimum Linux version is eventually bumped to the point where it's above 4.XX? Then, would this feature just become a no-op?

[SUPERCILEX]

Yeah, though I think we could actually just remove the feature since a MSRV bump is a breaking change anyway.

[SUPERCILEX]

Ok, I gave the feature gate idea a go: #501

[SUPERCILEX]

@sunfishcode Do you think it might be worth releasing 0.37 with what we've got and punting dup + other changes to 0.38? We've already accumulated 6 breaking changes which feels appropriate for a release: https://github.com/bytecodealliance/rustix/pulls?q=label%3A0.37

[sunfishcode]

Thanks for bringing this up. With #495 changing from being a definition-of-safety issue to merely being an ergonomics issue, I no longer thing it needs to block 0.37. And I think that's the last blocker we've identified here.

I've now released a 0.37.0-alpha.0 prerelease to get the process started. If anyone tries this version and finds a bug or something missing please report it!

I'm also hoping to catch up on reviews, and if I can do that soon enough then we can get them in before releasing 0.37 too.

[SUPERCILEX]

Awesome, thanks! I just upgraded SUPERCILEX/fuc@1c310e0 and it's looking good.

[Unpublished]

I'd like to see #536 as well in a 0.37 release

[magicse]

Hi there. Plese could you explain me. It this normal sitation with statx ?

statx(AT_FDCWD, "/usr/local/lib/python3.10/site-packages/numpy/core", AT_STATX_SYNC_AS_STAT, STATX_BASIC_STATS, 0x7dfec2d0) = -1 ENOSYS (Function not implemented)
stat64("/usr/local/lib/python3.10/site-packages/numpy/core", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
statx(AT_FDCWD, "/usr/local/lib/python3.10/site-packages/numpy/core/_multiarray_umath.cpython-310-arm-linux-gnueabihf.so", AT_STATX_SYNC_AS_STAT, STATX_BASIC_STATS, 0x7dfec0b0) = -1 ENOSYS (Function not implemented)
stat64("/usr/local/lib/python3.10/site-packages/numpy/core/_multiarray_umath.cpython-310-arm-linux-gnueabihf.so", {st_mode=S_IFREG|0755, st_size=2374876, ...}) = 0
open("/usr/local/lib/python3.10/site-packages/numpy/core/_multiarray_umath.cpython-310-arm-linux-gnueabihf.so", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 3
fcntl64(3, F_SETFD, FD_CLOEXEC)         = 0
statx(3, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_BASIC_STATS, 0x7dfebc50) = -1 ENOSYS (Function not implemented)
fstat64(3, {st_mode=S_IFREG|0755, st_size=2374876, ...}) = 0
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0(\0\1\0\0\0\0\0\0\0004\0\0\0"..., 936) = 936
mmap2(NULL, 2555904, PROT_READ|PROT_EXEC, MAP_PRIVATE, 3, 0) = 0x75420000
mmap2(0x75650000, 229376, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED, 3, 0x220000) = 0x75650000
mmap2(0x75670000, 98304, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x75670000
mmap2(0x75680000, 32768, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED, 3, 0x238000) = 0x75680000
mmap2(0x75680000, 65536, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED, 3, 0x240000) = 0x75680000
close(3)                                = 0
--- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=NULL} ---
+++ killed by SIGSEGV +++

[sunfishcode]

@magicse Are you using rustix?

[magicse]

I don't know. This is not my build. I get all time segfault and try strace... and this is fragment of strace log
Alpine Linux
I try simple import numpy in python and get sigfault.

[LingMan]

@magicse: Numpy is written in Python and C. Not in Rust. No clue what causes your problem, but you're talking to the wrong people.

[sunfishcode]

I've now released a v0.37.0-alpha.1 with all the PRs on my radar for including in the 0.37 release. If there's anything else we should include, please post about it here! I plan to do a 0.37 release soon.

[sunfishcode]

I've now released 0.37! See the release notes for a summary of new features and changes.