Skip to content

Commit c9df1d1

Browse files
committed
Do not chdir before calling posix_spawn
We allow users to set the working directory of the child process when they use the Process API. Unfortunately, this was implemented by calling chdir in the parent process, which has nasty global effects on the process and cannot safely be used concurrently. glibc has a non-POSIX extension to posix_spawn that can call chdir in the child process, so we add that call instead where it's available, and add a regression test.
1 parent cd98893 commit c9df1d1

File tree

2 files changed

+61
-0
lines changed

2 files changed

+61
-0
lines changed

Sources/Foundation/Process.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -938,6 +938,17 @@ open class Process: NSObject {
938938
}
939939
#endif
940940

941+
#if os(Linux)
942+
if let dir = currentDirectoryURL?.path {
943+
try _throwIfPosixError(posix_spawn_file_actions_addchdir_np(fileActions, dir))
944+
}
945+
#else
946+
// This is an unfortunate workaround: posix_spawn has no POSIX-specified way to set the working directory
947+
// of the child process. glibc has a non-POSIX API option, which we use above. Here we take a brute-force
948+
// approach of just changing our current working directory. This is not a great implementation and it's likely
949+
// to cause subtle issues in those environments. However, the Apple Foundation library doesn't have this problem,
950+
// and this library does the right thing on Linux and Windows, so the overwhelming majority of users are
951+
// well-served.
941952
let fileManager = FileManager()
942953
let previousDirectoryPath = fileManager.currentDirectoryPath
943954
if let dir = currentDirectoryURL?.path, !fileManager.changeCurrentDirectoryPath(dir) {
@@ -948,6 +959,7 @@ open class Process: NSObject {
948959
// Reset the previous working directory path.
949960
fileManager.changeCurrentDirectoryPath(previousDirectoryPath)
950961
}
962+
#endif
951963

952964
// Launch
953965
var pid = pid_t()

Tests/Foundation/Tests/TestProcess.swift

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
88
//
99

10+
import Dispatch
11+
1012
class TestProcess : XCTestCase {
1113

1214
func test_exit0() throws {
@@ -699,6 +701,53 @@ class TestProcess : XCTestCase {
699701
}
700702
}
701703

704+
func test_currentDirectoryDoesNotChdirParentProcess() throws {
705+
// This test only behaves correctly on Linux and Windows: other platforms don't have an
706+
// appropriate API for this in posix_spawn or similar.
707+
#if os(Linux) || os(Windows)
708+
let backgroundQueue = DispatchQueue(label: "background-processor")
709+
let group = DispatchGroup()
710+
let startSemaphore = DispatchSemaphore()
711+
let currentWorkingDirectory = FileManager.shared.currentDirectoryPath
712+
var shouldRun = true
713+
let shouldRunLock = Lock()
714+
715+
XCTAssertNotEqual(currentWorkingDirectory, "/")
716+
717+
// Kick off the background task. This will spin on our current working directory and confirm
718+
// it doesn't change.
719+
backgroundQueue.async(group: group) {
720+
startSemaphore.signal()
721+
722+
while true {
723+
let newCWD = FileManager.shared.currentDirectoryPath
724+
XCTAssertEqual(newCWD, currentWorkingDirectory)
725+
726+
shouldRunLock.lock()
727+
if shouldRun {
728+
shouldRunLock.unlock()
729+
} else {
730+
shouldRunLock.unlock()
731+
break
732+
}
733+
}
734+
}
735+
736+
startSemaphore.wait()
737+
738+
// We run the task 50 times just to try to encourage it to fail.
739+
for _ in 0..<50 {
740+
XCTAssertNoThrow(try runTask([xdgTestHelperURL().path, "--getcwd"], currentDirectoryPath: "/"))
741+
}
742+
743+
shouldRunLock.lock()
744+
shouldRun = false
745+
shouldRunLock.unlock()
746+
747+
group.wait()
748+
#endif
749+
}
750+
702751
#if !os(Windows)
703752
func test_fileDescriptorsAreNotInherited() throws {
704753
let task = Process()

0 commit comments

Comments
 (0)