Skip to content

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

Merged
merged 6 commits into from
Jun 9, 2025

Conversation

grynspan
Copy link
Contributor

@grynspan grynspan commented Jun 9, 2025

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. 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:

  • Code and documentation should follow the style of the Style Guide.
  • If public symbols are renamed or modified, DocC references should be updated.

…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.
@grynspan grynspan added this to the Swift 6.x milestone Jun 9, 2025
@grynspan grynspan self-assigned this Jun 9, 2025
@grynspan grynspan added bug 🪲 Something isn't working linux 🐧 Linux support (all distros) exit-tests ☠️ Work related to exit tests labels Jun 9, 2025
@grynspan
Copy link
Contributor Author

grynspan commented Jun 9, 2025

@swift-ci test

@grynspan
Copy link
Contributor Author

grynspan commented Jun 9, 2025

@swift-ci test

@grynspan
Copy link
Contributor Author

grynspan commented Jun 9, 2025

@swift-ci test

@grynspan grynspan marked this pull request as ready for review June 9, 2025 21:14
@jmschonfeld
Copy link

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)

@grynspan
Copy link
Contributor Author

grynspan commented Jun 9, 2025

@swift-ci test

@grynspan grynspan requested a review from stmontgomery June 9, 2025 21:57
@stmontgomery
Copy link
Contributor

@swift-ci Please test

@grynspan grynspan merged commit b0a1efd into main Jun 9, 2025
3 checks passed
@grynspan grynspan deleted the jgrynspan/1140-close-write-end branch June 9, 2025 23:08
@grynspan
Copy link
Contributor Author

Relevant glibc change (non-portable and only on relatively recent glibc versions though.) https://sourceware.org/bugzilla/show_bug.cgi?id=23640

@grynspan
Copy link
Contributor Author

FreeBSD also clears FD_CLOEXEC when you call posix_spawn_file_actions_adddup2(n, n) however I'll need to confirm what version implemented this functionality before I can rely on it.

@grynspan
Copy link
Contributor Author

@grynspan
Copy link
Contributor Author

@grynspan
Copy link
Contributor Author

grynspan added a commit that referenced this pull request Jun 10, 2025
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working exit-tests ☠️ Work related to exit tests linux 🐧 Linux support (all distros)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exit tests hang indefinitely on Ubuntu 20.04
3 participants