Skip to content

Conversation

jakepetroules
Copy link
Contributor

@jakepetroules jakepetroules commented Aug 21, 2025

A couple tests attempt to double-close a file descriptor. You cannot ever safely do this as even the act of spawning a process itself may have re-allocated the same file descriptor in the calling process - for example, the pidfd file descriptor, or other threads may have changed state. Disable these tests until we figure out what to do with them. It's possible the solution may involve #161, where Subprocess's own FileDescriptor type can have its own logic for refusing to double-close, and we validate the condition at that level rather than at the POSIX level.

Also moves some other code to using closeAfter to guarantee proper cleanup.

@iCharlesHu
Copy link
Contributor

Failures seem related?

A couple tests attempt to double-close a file descriptor. You cannot ever safely do this as even the act of spawning a process itself may have re-allocated the same file descriptor in the calling process - for example, the pidfd file descriptor, or other threads may have changed state. Disable these tests until we figure out what to do with them. It's possible the solution may involve swiftlang#161, where Subprocess's own FileDescriptor type can have its own logic for refusing to double-close, and we validate the condition at that level rather than at the POSIX level.

This hasn't failed in CI so far due to the older Linux kernel in use.

Also moves some other code to using closeAfter to guarantee proper cleanup.
@jakepetroules jakepetroules changed the title Isolate tests which double-close file descriptors using exit tests Disable tests which double-close file descriptors Aug 21, 2025
@jakepetroules jakepetroules merged commit 7dc6e54 into swiftlang:main Aug 21, 2025
39 checks passed
@jakepetroules jakepetroules deleted the eng/PR-double-close branch August 21, 2025 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants