dup2 and close potentially should be unsafe

[RalfJung]

See the discussion in rust-lang/rust#72175, in particular this comment: dup2 and close (and potentially more operations) can be used to destroy file descriptors that might be owned by other libraries, which in turn can lead to internal invariants of those libraries being violated. For this reason, the standard library makes from_raw_fd an unsafe operation. nix should, I think, follow suit and not expose safe operations that break the idea that a file descriptor can be exclusively owned.

[asomers]

It's a tough call. On the one hand, Nix usually defers to rust-lang's reasoning on safety. If rust-lang says that from_raw_fd is unsafe, then anything equivalent in Nix should be, too. On the other hand, dup2 doesn't implement FromRawFd because it doesn't return a File; it returns a different RawFd. And yes, doing something like closing the returned RawFd could crash a different library, but so could nix::unistd::close. But for that matter, nix::unistd::{write, lseek, ftruncate, ...} could also cause another library's invariants to be violated. Maybe RawFd should be treated like a raw pointer; any dereference is inherently unsafe. If not, then it should be treated like "special operating system stuff": beyond the scope of the Rust language. In that case, nothing involving RawFd should be considered unsafe. In either case, I don't see a good argument for making just dup2 unsafe.

[RalfJung]

In either case, I don't see a good argument for making just dup2 unsafe.

That is absolutely fair. I already mentioned close in my issue; I was simply not aware of the entire API surface nix provides -- indeed, to match from_raw_fd being unsafe, every operation on a file descriptor that "might not be owned" should be unsafe. Or, alternatively, those operations could take a type like OwnedFd and the RawFd → OwnedFd conversion should be unsafe (this would more precisely reflect the safety story and probably reduce unsafe code in clients).

[sunfishcode]

I'm working on a draft of a proposal to add new official wording about RawFd to Rust.

In particular, this proposal would mean that all functions with a RawFd as an argument should be marked unsafe, or migrated to something like OwnedFd. I recognize this would be a major change for nix, but I'm hoping the benefits of having clearer guarantees outweigh the costs. I'd be interested in any feedback from folks working on nix!

[asomers]

I'm going to close this issue. But @sunfishcode please open a new one if that RFC ever gets fully implemented. Nix should definitely follow suit.