Darwin: Only set FD_CLOEXEC when creating a socket FD

[carlhoerberg]

Not when an existing FD is passed to Socket.new

[carlhoerberg]

Follow up on #14632 (review)

[ysbaddaden]

I notice some issues with this change: when we create the socket the @handle ivar isn't set and calling #fcntl will fail (see CI) but we also set CLOEXEC for plain fd (e.g. Socket.new(fd, ...) as called from UNIXSocket.pair or Socket#accept? for example.

[straight-shoota]

when we create the socket the @handle ivar isn't set and calling #fcntl will fail (see CI)

Yeah I think we need to use LibC.fcntl(fd, ...) like before to fix the current failure. The file descriptor is not assigned yet, so we cannot use #fcntl.

we also set CLOEXEC for plain fd (e.g. Socket.new(fd, ...) as called from UNIXSocket.pair or Socket#accept? for example.

Yes, that's the current behaviour. But I believe it's wrong. We probably shouldn't autoclose a file descriptor that's coming from outside the process. Only close what we opened ourselves.
That means all uses in stdlib like Socket#accept? need to be updated to explicitly set close on exec.

However, I'm wondering if this isn't too bad a silent breaking change in behaviour... 🤔

[ysbaddaden]

I agree: it may not be the best API, but security-wise it doesn't sound like a bad practice to always close file descriptors on exec... and maybe always calling fcntl ain't a bad idea instead of setting O_CLOEXEC after all? If you need it to not be closed on exec, you can always call fcntl to disable the flag.

[ysbaddaden]

The following methods (at least) must also be fixed to set FD_CLOEXEC for darwin targets:

• Socket#accept?
• UNIXSocket#accept?
• UNIXSocket.pair

Maybe there should be a system method to set FD_CLOEXEC on a given fd to avoid repeating the same LibC.fcntl call? Something like Crystal::System::FileDescriptor.set_close_on_exec(fd : Int32) : Nil.

Why Apple wouldn't implement the O_CLOEXEC SOCK_CLOEXEC flag is beyond me 😮‍💨

[straight-shoota]

Yeah, I suppose we should also setup specs for those methods.

@carlhoerberg If you want to go on, please continue. But we'll be happy to take over these final steps if you prefer. Just say the word.

[carlhoerberg]

Go for it! :)