Skip to content

Commit 22b7ad6

Browse files
committed
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.
1 parent 93fd967 commit 22b7ad6

File tree

3 files changed

+68
-4
lines changed

3 files changed

+68
-4
lines changed

Sources/Testing/ExitTests/SpawnProcess.swift

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -284,25 +284,47 @@ func spawnExecutable(
284284
let commandLine = _escapeCommandLine(CollectionOfOne(executablePath) + arguments)
285285
let environ = environment.map { "\($0.key)=\($0.value)" }.joined(separator: "\0") + "\0\0"
286286

287+
// CreateProcessW() may modify the command line argument, so we must make
288+
// a mutable copy of it. (environ is also passed as a mutable raw pointer,
289+
// but it is not documented as actually being mutated.)
290+
let commandLineCopy = commandLine.withCString(encodedAs: UTF16.self) { _wcsdup($0) }
291+
defer {
292+
free(commandLineCopy)
293+
}
294+
295+
// On Windows, a process holds a reference to its current working
296+
// directory, which prevents other processes from deleting it. This causes
297+
// code to fail if it tries to set the working directory to a temporary
298+
// path. SEE: https://github.com/swiftlang/swift-testing/issues/1209
299+
//
300+
// This problem manifests for us when we spawn a child process without
301+
// setting its working directory, which causes it to default to that of
302+
// the parent process. To avoid this problem, we set the working directory
303+
// of the new process to the root directory of the boot volume (which is
304+
// unlikely to be deleted, one hopes).
305+
//
306+
// SEE: https://devblogs.microsoft.com/oldnewthing/20101109-00/?p=12323
307+
let workingDirectoryPath = rootDirectoryPath
308+
287309
var flags = DWORD(CREATE_NO_WINDOW | CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT)
288310
#if DEBUG
289311
// Start the process suspended so we can attach a debugger if needed.
290312
flags |= DWORD(CREATE_SUSPENDED)
291313
#endif
292314

293-
return try commandLine.withCString(encodedAs: UTF16.self) { commandLine in
294-
try environ.withCString(encodedAs: UTF16.self) { environ in
315+
return try environ.withCString(encodedAs: UTF16.self) { environ in
316+
try workingDirectoryPath.withCString(encodedAs: UTF16.self) { workingDirectoryPath in
295317
var processInfo = PROCESS_INFORMATION()
296318

297319
guard CreateProcessW(
298320
nil,
299-
.init(mutating: commandLine),
321+
commandLineCopy,
300322
nil,
301323
nil,
302324
true, // bInheritHandles
303325
flags,
304326
.init(mutating: environ),
305-
nil,
327+
workingDirectoryPath,
306328
startupInfo.pointer(to: \.StartupInfo)!,
307329
&processInfo
308330
) else {

Sources/Testing/Support/FileHandle.swift

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,4 +719,35 @@ func setFD_CLOEXEC(_ flag: Bool, onFileDescriptor fd: CInt) throws {
719719
}
720720
}
721721
#endif
722+
723+
/// The path to the root directory of the boot volume.
724+
///
725+
/// On Windows, this string is usually of the form `"C:\"`. On UNIX-like
726+
/// platforms, it is always equal to `"/"`.
727+
let rootDirectoryPath: String = {
728+
#if os(Windows)
729+
var result: String?
730+
731+
// The boot volume is, except in some legacy scenarios, the volume that
732+
// contains the system Windows directory. For an explanation of the difference
733+
// between the Windows directory and the _system_ Windows directory, see
734+
// https://devblogs.microsoft.com/oldnewthing/20140723-00/?p=423 .
735+
let count = GetSystemWindowsDirectoryW(nil, 0)
736+
if count > 0 {
737+
withUnsafeTemporaryAllocation(of: wchar_t.self, capacity: Int(count) + 1) { buffer in
738+
_ = GetSystemWindowsDirectoryW(buffer.baseAddress!, UINT(buffer.count))
739+
let rStrip = PathCchStripToRoot(buffer.baseAddress!, buffer.count)
740+
if rStrip == S_OK || rStrip == S_FALSE {
741+
result = String.decodeCString(buffer.baseAddress!, as: UTF16.self)?.result
742+
}
743+
}
744+
}
745+
746+
// If we weren't able to get a path, fall back to "C:\" on the assumption that
747+
// it's the common case and most likely correct.
748+
return result ?? #"C:\"#
749+
#else
750+
return "/"
751+
#endif
752+
}()
722753
#endif

Tests/TestingTests/Support/FileHandleTests.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,17 @@ struct FileHandleTests {
201201
#endif
202202
}
203203
#endif
204+
205+
@Test("Root directory path is correct")
206+
func rootDirectoryPathIsCorrect() throws {
207+
#if os(Windows)
208+
if let systemDrive = Environment.variable(named: "SYSTEMDRIVE") {
209+
#expect(rootDirectoryPath.starts(with: systemDrive))
210+
}
211+
#else
212+
#expect(rootDirectoryPath == "/")
213+
#endif
214+
}
204215
}
205216

206217
// MARK: - Fixtures

0 commit comments

Comments
 (0)