From 22b7ad6d37a287f566bb2d88afcb187fb8b727c5 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 8 Jul 2025 15:15:08 -0400 Subject: [PATCH] Reset the working directory of child processes we spawn on Windows. (#1212) This PR modifies the Windows implementation of `spawnProcess()` so that it sets the working directory of the new process to "C:\" (or thereabouts). This prevents a race condition on Windows because that system won't let you delete a directory if it's the working directory of any process. See [The Old New Thing](https://devblogs.microsoft.com/oldnewthing/20101109-00/?p=12323) for a very on-the-nose blog post. Note that we do not specify the value of the working directory in an exit test body. A test should generally not rely on it anyway because it is global state and any thread could change its value at any time. I haven't written a unit test for this change because it's unclear what I could write that would be easily verifiable, and because I don't know what state I might perturb outside such a test by calling `SetCurrentDirectory()`. Resolves #1209. (This is a speculative fix.) ### 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. --- Sources/Testing/ExitTests/SpawnProcess.swift | 30 +++++++++++++++--- Sources/Testing/Support/FileHandle.swift | 31 +++++++++++++++++++ .../Support/FileHandleTests.swift | 11 +++++++ 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/Sources/Testing/ExitTests/SpawnProcess.swift b/Sources/Testing/ExitTests/SpawnProcess.swift index fe51a7086..9f01a1d11 100644 --- a/Sources/Testing/ExitTests/SpawnProcess.swift +++ b/Sources/Testing/ExitTests/SpawnProcess.swift @@ -284,25 +284,47 @@ func spawnExecutable( let commandLine = _escapeCommandLine(CollectionOfOne(executablePath) + arguments) let environ = environment.map { "\($0.key)=\($0.value)" }.joined(separator: "\0") + "\0\0" + // CreateProcessW() may modify the command line argument, so we must make + // a mutable copy of it. (environ is also passed as a mutable raw pointer, + // but it is not documented as actually being mutated.) + let commandLineCopy = commandLine.withCString(encodedAs: UTF16.self) { _wcsdup($0) } + defer { + free(commandLineCopy) + } + + // On Windows, a process holds a reference to its current working + // directory, which prevents other processes from deleting it. This causes + // code to fail if it tries to set the working directory to a temporary + // path. SEE: https://github.com/swiftlang/swift-testing/issues/1209 + // + // This problem manifests for us when we spawn a child process without + // setting its working directory, which causes it to default to that of + // the parent process. To avoid this problem, we set the working directory + // of the new process to the root directory of the boot volume (which is + // unlikely to be deleted, one hopes). + // + // SEE: https://devblogs.microsoft.com/oldnewthing/20101109-00/?p=12323 + let workingDirectoryPath = rootDirectoryPath + var flags = DWORD(CREATE_NO_WINDOW | CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT) #if DEBUG // Start the process suspended so we can attach a debugger if needed. flags |= DWORD(CREATE_SUSPENDED) #endif - return try commandLine.withCString(encodedAs: UTF16.self) { commandLine in - try environ.withCString(encodedAs: UTF16.self) { environ in + return try environ.withCString(encodedAs: UTF16.self) { environ in + try workingDirectoryPath.withCString(encodedAs: UTF16.self) { workingDirectoryPath in var processInfo = PROCESS_INFORMATION() guard CreateProcessW( nil, - .init(mutating: commandLine), + commandLineCopy, nil, nil, true, // bInheritHandles flags, .init(mutating: environ), - nil, + workingDirectoryPath, startupInfo.pointer(to: \.StartupInfo)!, &processInfo ) else { diff --git a/Sources/Testing/Support/FileHandle.swift b/Sources/Testing/Support/FileHandle.swift index 1c5447460..37774b91a 100644 --- a/Sources/Testing/Support/FileHandle.swift +++ b/Sources/Testing/Support/FileHandle.swift @@ -719,4 +719,35 @@ func setFD_CLOEXEC(_ flag: Bool, onFileDescriptor fd: CInt) throws { } } #endif + +/// The path to the root directory of the boot volume. +/// +/// On Windows, this string is usually of the form `"C:\"`. On UNIX-like +/// platforms, it is always equal to `"/"`. +let rootDirectoryPath: String = { +#if os(Windows) + var result: String? + + // The boot volume is, except in some legacy scenarios, the volume that + // contains the system Windows directory. For an explanation of the difference + // between the Windows directory and the _system_ Windows directory, see + // https://devblogs.microsoft.com/oldnewthing/20140723-00/?p=423 . + let count = GetSystemWindowsDirectoryW(nil, 0) + if count > 0 { + withUnsafeTemporaryAllocation(of: wchar_t.self, capacity: Int(count) + 1) { buffer in + _ = GetSystemWindowsDirectoryW(buffer.baseAddress!, UINT(buffer.count)) + let rStrip = PathCchStripToRoot(buffer.baseAddress!, buffer.count) + if rStrip == S_OK || rStrip == S_FALSE { + result = String.decodeCString(buffer.baseAddress!, as: UTF16.self)?.result + } + } + } + + // If we weren't able to get a path, fall back to "C:\" on the assumption that + // it's the common case and most likely correct. + return result ?? #"C:\"# +#else + return "/" +#endif +}() #endif diff --git a/Tests/TestingTests/Support/FileHandleTests.swift b/Tests/TestingTests/Support/FileHandleTests.swift index 4be633ad6..acca1dbea 100644 --- a/Tests/TestingTests/Support/FileHandleTests.swift +++ b/Tests/TestingTests/Support/FileHandleTests.swift @@ -201,6 +201,17 @@ struct FileHandleTests { #endif } #endif + + @Test("Root directory path is correct") + func rootDirectoryPathIsCorrect() throws { +#if os(Windows) + if let systemDrive = Environment.variable(named: "SYSTEMDRIVE") { + #expect(rootDirectoryPath.starts(with: systemDrive)) + } +#else + #expect(rootDirectoryPath == "/") +#endif + } } // MARK: - Fixtures