swiftlang / swift-corelibs-foundation

The Foundation Project, providing core utilities, internationalization, and OS independence
swift.org
Apache License 2.0
5.27k stars 1.13k forks source link

[SR-15471] Race condition when launching Process on Linux can leak file descriptors #3946

Open swift-ci opened 2 years ago

swift-ci commented 2 years ago
Previous ID SR-15471
Radar None
Original Reporter george (JIRA User)
Type Bug
Environment Any
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Foundation | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: 3ed8fbdf2dbfa24098e0dc3fa558bb98

Issue Description:

The code for launching a `Process` on Linux emulates the Apple-specific extensions POSIX_SPAWN_CLOEXEC_DEFAULT by iterating over the currently open file descriptors and adding them to the posix_file_actions_t. However, if a descriptor is opened from a different thread after iteration begins, but before `posx_spawn` is called, it is possible that it will not be closed. This could lead to a deadlock if a child process waits on a file descriptor that has unintentionally leaked into that child process.

The offending code is here: https://github.com/apple/swift-corelibs-foundation/blob/08979ae37953be8d8b2d75622f16de33f274939c/Sources/Foundation/Process.swift#L957

In one of my projects, I have worked around this by reimplementing this functionality using `clone` and doing the file descriptor iteration in the cloned process (which is guaranteed to be single-threaded): https://github.com/GeorgeLyon/Shwift/blob/f92f0c599a4294f9e49fa482fc2fabba0cbe8011/Sources/CLinuxSupport/CLinuxSupport_Parent.c#L213

GeorgeLyon commented 2 years ago

I did a more complete writeup of this issue in a post on the Swift forums: https://forums.swift.org/t/meet-swiftcommand-a-new-swift-package-that-makes-creating-child-processes-very-easy/59701/6