-
Notifications
You must be signed in to change notification settings - Fork 110
Disable inheritance of file descriptors created by Swift Testing by default. #1145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…efault. This PR makes file descriptors created by Swift Testing `FD_CLOEXEC` by default (on Windows, `~HANDLE_FLAG_INHERIT`.) We then clear `FD_CLOEXEC` for specific file descriptors that should be inherited. On Darwin, this is effectively ignored because we use `POSIX_SPAWN_CLOEXEC_DEFAULT`, and on Windows we use `PROC_THREAD_ATTRIBUTE_HANDLE_LIST` which has much the same effect. (On non-Darwin POSIX platforms, there's no reliable way to ensure only one child process inherits a particular file descriptor.) This change is speculative. Resolves #1140.
@swift-ci test |
@swift-ci test |
@swift-ci test |
Confirmed that a swift-foundation branch with exit tests enabled successfully ran those exit tests with this swift-testing version on Ubuntu 20.04 (but failed previously) |
@swift-ci test |
@swift-ci Please test |
Relevant glibc change (non-portable and only on relatively recent glibc versions though.) https://sourceware.org/bugzilla/show_bug.cgi?id=23640 |
FreeBSD also clears |
On [FreeBSD](https://man.freebsd.org/cgi/man.cgi?query=posix_spawn_file_actions_adddup2), [OpenBSD](https://github.com/openbsd/src/blob/master/lib/libc/gen/posix_spawn.c#L155), [Android](https://android.googlesource.com/platform/bionic/+/master/libc/bionic/spawn.cpp#103), and [Glibc ≥ 2.29](https://sourceware.org/bugzilla/show_bug.cgi?id=23640), `posix_spawn_file_actions_adddup2()` automatically clears `FD_CLOEXEC` if the file descriptors passed to it are equal. Relying on this behaviour eliminates a race condition when spawning child processes. This functionality is standardized in [POSIX.1-2024](https://pubs.opengroup.org/onlinepubs/9799919799/) thanks to [Austin Group Defect #411](https://www.austingroupbugs.net/view.php?id=411). Some older Linuxes (Amazon Linux 2 in particular) don't have this functionality, so we do a runtime check of the Glibc version. This PR is a follow-up to #1145. Resolves #1140. ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
This PR makes file descriptors created by Swift Testing
FD_CLOEXEC
by default (on Windows,~HANDLE_FLAG_INHERIT
.) We then clearFD_CLOEXEC
for specific file descriptors that should be inherited. On Darwin, this is effectively ignored because we usePOSIX_SPAWN_CLOEXEC_DEFAULT
, and on Windows we usePROC_THREAD_ATTRIBUTE_HANDLE_LIST
which has much the same effect. (On non-Darwin POSIX platforms, there's no reliable way to ensure only one child process inherits a particular file descriptor.)This change is speculative. No additional unit tests are added because existing test coverage should be sufficient; the reported issue is on a platform (Ubuntu 20.04) where we don't have any CI jobs.
Resolves #1140.
Checklist: