Add Sendable conformance to FileDescriptor

[ffried]

This adds Sendable conformance to FileDescriptor alongside the other public types (see #115).

[Lukasa]

I'm not convinced FileDescriptor should be Sendable: it has a bunch of global state that can absolutely misbehave if you concurrently use it from multiple threads.

[ffried]

Oh, good catch!
Should I annotate the conformance as @available(*, unavailable) to make this explicit?

[ffried]

I think it would make sense. The correct way would be to pass the FilePath across threads and open a FileDescriptor on each thread that needs one.

I've added the annotation.

[milseman]

We certainly don't want to encourage that as normal practice. This does need to be balanced with the fact that System exposes the underlying system interfaces and capabilities as-is. On the system, file descriptors and open file descriptions are very much shared and sharable across threads.

I.e. it should be possible to write a high quality database in Swift using System and careful use of file locks (eventually coming in #111).

A conservative approach is to keep the conformance off of FileDescriptor and let the developer explicitly pass rawValues around.

[ffried]

@milseman I completely agree with your reasoning here. What I'm still unsure about is, how this will behave in future Swift versions. I'm not too familiar with the plans there, but AFAIK Swift 6 will warn when passing around non-Sendable objects across async contexts. Is there a difference between not conforming FileDescriptor to Sendable and explicitly making it "non-Sendable"?
The effect would still be the same (having to pass around rawValue), or am I missing something here?

Note that I don't have an issue with removing the conformance altogether. I'm wondering whether this is (or will be) actually different from explicitly marking it as unavailable.

[ffried]

@milseman Thank you for this in-depth explanation!

I'm not sure if I can provide any useful input to this decision. Even - or especially - with your write-up, I still see both options as reasonable. Either option will need a proper documentation explaining the decision and its consequences.

I'd like to debate/stew over this a little more and figure out how it could fit in with System's philosophy, especially as we see how file locks, descriptors shared across fork\exec, etc. work out.

Would it make sense to factor the conformance of FileDescriptor out into a separate PR for this?
Or will this just create confusion in case a release is finalized w/o the conformance decision for FileDescriptor?

Either way, should I just leave this (or the new PR) open then? And should I ping you guys from time to time, or is this tracked internally anyways?

[MaxDesiatov]

Sorry for interjecting, but there are other projects that would benefit from FilePath: Sendable conformance, so I personally hope at least that part could land soon-ish if there are no objections to it.

[milseman]

I'm fine splitting FileDescriptor's decision off or keeping this open, your call.

Another useful point, would socket descriptors be sendable @Lukasa? Our prototype had a different type for sockets with explicit conversions between file and socket descriptors. Similarly, read-ends of pipes, etc. We might end up with a more fleshed out descriptor story with multiple types.

[Lukasa]

That's a good question with a tricky answer. By the same logic that file descriptors aren't, socket descriptors aren't (calling read from multiple threads necessarily has unexpected effects). Ideally we'd be able to move it across threads but not to share the reference. This would encourage users to dup the FD, which has substantially clearer semantics.

[ffried]

I've split off the other conformances in #115. This PR now only adds the FileDescriptor conformance (on top of #115).

[milseman]

I'd like to reengage with this topic. I'm still (even more) on the side of FileDescriptor being Sendable. @lorentey what would be the ABI impact of adding that conformance?

[lorentey]

Sendable has no ABI impact -- we can simply add the conformance whenever we are ready!

[ffried]

@milseman @lorentey Should I change this PR to add the conformance (w/o marking it as unavailable)?

[milseman]

Yes to adding it, deferring to @lorentey regarding availability.

[ffried]

In accordance with the change in 87e68fc, I've updated the conformance to be on the main type. This also resolves the conflicts that existed on this PR's branch.

Depending on @lorentey's decision regarding availability, I'll move it to an extension again (for marking it unavailable).

[lorentey]

Update: As a @frozen struct wrapping a single integer, the shipping FileDescriptor has already been implicitly inferred to be Sendable, as it should be.

Concurrent use of the same file descriptor does not lead to undefined behavior. And even if it did, it is not in Swift System's mandate to protect against that.

The job of Swift System is merely to present the preexisting C APIs in a way that feels native to Swift. This codebase is not in the business of inventing nontrivial abstractions over the constructs provided by the operating system, or of building pretend obstacles against their use. One can pass integer values between concurrent execution contexts and directly call the read/write/close/etc. syscalls on them -- therefore, it must be possible to do the same on FileDescriptor instances and their operations.

[ffried]

@lorentey Thanks for the update. In this case this PR should be ready as is, right? I've just rebased the branch on main to make sure there are no conflicts.

[lorentey]

@swift-ci test