asio
asio copied to clipboard
chriskohlhoff
Provide a way to specify CLOEXEC flags on file descriptor construction
There seems to be no way to specify options on file descriptor construction, where a CLOEXEC flag could be specified.
This primarily concerns sockets and SOCK_CLOEXEC flag for socket and accept4 system calls. While for socket construction this can be worked around by user providing a pre-created socket file descriptor, this is not possible to do for accepting incoming connections - basic_socket_acceptor::accept does not take any options and does not use accept4 internally. I think, the socket construction and connection interfaces should provide a way to specify options like SOCK_CLOEXEC. Similarly, if there are other kinds of file descriptors created in the library, they should support this as well.
I think this should be the default even. Leaking these sockets can't be safe without asio's fork hooks, and if one does want to use the fork hooks, they should take care of explicitly allowing the inheritance of the socket descriptors in child processes
Just my two cents on the matter:
- Currently, asio's notify_fork isn't async-signal-safe which causes undefined behavior.
- Setting FD_CLOEXEC via fnctl is impossible without a race condition.
At the least, the other UNIX-like systems provided closefrom, close_range, or some fnctl method. Linux developers literally just refused, expecting everyone to set CLOEXEC flags for every single file descriptor in the application, ignoring the fact that libraries (like ASIO) do not give you the ability to set flags for certain operations the create new descriptors. And because of this, it creates a new race condition every single time fnctl is called to set CLOEXEC flags.
POSIX itself sort of perpetuates the problem. accept() creates a file descriptor that can be inherited but doesn't provide a way to tell it to set close-on-exec flags during creation. This is the reason why accept4, paccept, etc. were created.
So the Right(TM) way is to not use libraries that don't let you control creation flags and make sure that you're setting whatever would cause close-on-exec. The Almost Right(TM) way would be to just call closefrom() in the forked process.
For Windows, things aren't much better but at least there, there's a way to handle everything. Every call to WinSock 2 has a way of avoid race conditions, there's a way to just turn off handle inheritance altogether, etc.
TL;DR: The issue is more complicated than most believe. However, ASIO should still provide a way to pass close-on-exec flags at the least since just about every platform has managed to provide them in some form or fashion for the purpose of avoiding race conditions in multi-threaded environment. ASIO is ironically not very multi-threaded friendly if the application ever forks at a non-deterministic time.
At the least, the other UNIX-like systems provided closefrom, close_range, or some fnctl method. Linux developers literally just refused, expecting everyone to set CLOEXEC flags for every single file descriptor in the application
To be fair, Linux does have close_range and closefrom (the latter - with libbsd). But I do still consider setting CLOEXEC the right way to go as this allows for a more portable and efficient code, because users won't have to deal with close_range and similar code.
At the least, the other UNIX-like systems provided closefrom, close_range, or some fnctl method. Linux developers literally just refused, expecting everyone to set CLOEXEC flags for every single file descriptor in the application
To be fair, Linux does have
close_rangeandclosefrom(the latter - with libbsd). But I do still consider settingCLOEXECthe right way to go as this allows for a more portable and efficient code, because users won't have to deal withclose_rangeand similar code.
close_range as a system call wasn't added until kernel 5.13 which is from late 2021. Most people still can't use these system calls. glibc does this to work around the problem, where it walks over /proc/self/fd/ and closes them. You'll sometimes find people just iterating using _SC_OPEN_MAX which is unfortunate since that can be in the hundreds of millions causing your application to effectively hang.
I do think setting CLOEXEC ideal but sometimes ideal isn't possible unfortunately.
close_range as a system call wasn't added until kernel 5.13 which is from late 2021.
It was added in 5.9 in October 2020, which is fairly wide spread now. The glibc wrapper was added later, in 2.34 in August 2021.
I think this should be the default even.
+1 for setting the CLOEXEC flag by default, especially that even currently Asio does attempt to do this in a few places.
The problem is that it tries to do this via ::fcntl(fd, F_SETFD, FD_CLOEXEC), which is obviously racy with a fork() call in another thread.
An external library doing fork() + exec() should not have to guess which file descriptors it needs to close before and it should not be in charge of taking care of Asio fd leaks.
