Skip to content

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.
owenv added a commit to owenv/swift-testing that referenced this pull request Jun 16, 2025
@stmontgomery stmontgomery modified the milestones: Swift 6.x, Swift 6.2 Jun 25, 2025
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

4 participants