Do not chdir before calling posix_spawn (#4606)

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.

Resolves swiftlang/swift#59610.

Author: Lukasa

CoreFoundation/Base.subproj/CFPlatform.c

@@ -2210,6 +2210,10 @@ CF_EXPORT int _CFPosixSpawnFileActionsAddClose(_CFPosixSpawnFileActionsRef file_
   return posix_spawn_file_actions_addclose((posix_spawn_file_actions_t *)file_actions, filedes);
 }
 
+CF_EXPORT int _CFPosixSpawnFileActionsAddChdirNP(_CFPosixSpawnFileActionsRef file_actions, const char *path) {
+  return posix_spawn_file_actions_addchdir_np((posix_spawn_file_actions_t *)file_actions, path);
+}
+
 CF_EXPORT int _CFPosixSpawn(pid_t *_CF_RESTRICT pid, const char *_CF_RESTRICT path, _CFPosixSpawnFileActionsRef file_actions, _CFPosixSpawnAttrRef _Nullable _CF_RESTRICT attrp, char *_Nullable const argv[_Nullable _CF_RESTRICT], char *_Nullable const envp[_Nullable _CF_RESTRICT]) {
   return posix_spawn(pid, path, (posix_spawn_file_actions_t *)file_actions, (posix_spawnattr_t *)attrp, argv, envp);
 }

CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h

@@ -709,6 +709,10 @@ CF_EXPORT int _CFPosixSpawn(pid_t *_CF_RESTRICT pid, const char *_CF_RESTRICT pa
 #else
 CF_EXPORT int _CFPosixSpawn(pid_t *_CF_RESTRICT pid, const char *_CF_RESTRICT path, _CFPosixSpawnFileActionsRef file_actions, _CFPosixSpawnAttrRef _Nullable _CF_RESTRICT attrp, char *_Nullable const argv[_Nullable _CF_RESTRICT], char *_Nullable const envp[_Nullable _CF_RESTRICT]);
 #endif // __cplusplus
+
+#if TARGET_OS_LINUX
+CF_EXPORT int _CFPosixSpawnFileActionsAddChdirNP(_CFPosixSpawnFileActionsRef file_actions, const char *path);
+#endif // TARGET_OS_LINUX
 #endif // !TARGET_OS_WIN32
 
 _CF_EXPORT_SCOPE_END

Sources/Foundation/Process.swift

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

Tests/Foundation/Tests/TestProcess.swift

@@ -7,6 +7,8 @@
 // See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 
+import Dispatch
+
 class TestProcess : XCTestCase {
 
     func test_exit0() throws {
@@ -699,6 +701,55 @@ class TestProcess : XCTestCase {
         }
     }
 
+    func test_currentDirectoryDoesNotChdirParentProcess() throws {
+        // This test only behaves correctly on Linux and Windows: other platforms don't have an
+        // appropriate API for this in posix_spawn or similar.
+        #if os(Linux) || os(Windows)
+        let backgroundQueue = DispatchQueue(label: "background-processor")
+        let group = DispatchGroup()
+        let startSemaphore = DispatchSemaphore(value: 0)
+        let currentWorkingDirectory = FileManager.default.currentDirectoryPath
+        var shouldRun = true
+        let shouldRunLock = NSLock()
+
+        XCTAssertNotEqual(currentWorkingDirectory, "/")
+
+        // Kick off the background task. This will spin on our current working directory and confirm
+        // it doesn't change.
+        backgroundQueue.async(group: group) {
+            startSemaphore.signal()
+
+            while true {
+                let newCWD = FileManager.default.currentDirectoryPath
+                XCTAssertEqual(newCWD, currentWorkingDirectory)
+
+                shouldRunLock.lock()
+                if shouldRun {
+                    shouldRunLock.unlock()
+                } else {
+                    shouldRunLock.unlock()
+                    break
+                }
+            }
+        }
+
+        startSemaphore.wait()
+
+        // We run the task 50 times just to try to encourage it to fail.
+        for _ in 0..<50 {
+            XCTAssertNoThrow(try runTask([xdgTestHelperURL().path, "--getcwd"], currentDirectoryPath: "/"))
+        }
+
+        shouldRunLock.lock()
+        shouldRun = false
+        shouldRunLock.unlock()
+
+        group.wait()
+        #else
+        throw XCTSkip()
+        #endif
+    }
+
     #if !os(Windows)
     func test_fileDescriptorsAreNotInherited() throws {
         let task = Process()
@@ -866,6 +917,7 @@ class TestProcess : XCTestCase {
             ("test_currentDirectory", test_currentDirectory),
             ("test_pipeCloseBeforeLaunch", test_pipeCloseBeforeLaunch),
             ("test_multiProcesses", test_multiProcesses),
+            ("test_currentDirectoryDoesNotChdirParentProcess", test_currentDirectoryDoesNotChdirParentProcess),
         ]
 
 #if !os(Windows)