Skip to content

Commit b8ad565

Browse files
committed
Fix process handle leak on Windows
- Add a missing CloseHandle as part of the process termination handling - Enable Process tests on Windows
1 parent 94ef6ab commit b8ad565

File tree

5 files changed

+21
-13
lines changed

5 files changed

+21
-13
lines changed

Package.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ let package = Package(
349349
"FoundationNetworking",
350350
"XCTest",
351351
"Testing",
352-
.target(name: "xdgTestHelper", condition: .when(platforms: [.linux, .android]))
352+
.target(name: "xdgTestHelper", condition: .when(platforms: [.linux, .android, .windows]))
353353
],
354354
resources: [
355355
.copy("Foundation/Resources")

Sources/Foundation/Process.swift

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1198,10 +1198,21 @@ open class Process: NSObject, @unchecked Sendable {
11981198
}
11991199

12001200
if let handler = self.terminationHandler {
1201-
let thread: Thread = Thread { handler(self) }
1201+
let thread: Thread = Thread {
1202+
handler(self)
1203+
self.closeHandle()
1204+
}
12021205
thread.start()
1206+
} else {
1207+
closeHandle()
12031208
}
12041209
}
1210+
1211+
private func closeHandle() {
1212+
#if os(Windows)
1213+
CloseHandle(self.processHandle)
1214+
#endif
1215+
}
12051216
}
12061217

12071218
extension Process {

Tests/Foundation/TestBundle.swift

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,7 @@ internal func testBundleName() -> String {
4343
}
4444

4545
internal func xdgTestHelperURL() throws -> URL {
46-
#if os(Windows)
47-
// Adding the xdgTestHelper as a dependency of TestFoundation causes its object files (including the main function) to be linked into the test runner executable as well
48-
// While this works on Linux due to special linker functionality, this doesn't work on Windows and results in a collision between the two main symbols
49-
// SwiftPM also cannot support depending on this executable (to ensure it is built) without also linking its objects into the test runner
50-
// For those reasons, using the xdgTestHelper on Windows is currently unsupported and tests that rely on it must be skipped
51-
throw XCTSkip("xdgTestHelper is not supported during testing on Windows (test executables are not supported by SwiftPM on Windows)")
52-
#else
5346
testBundle().bundleURL.deletingLastPathComponent().appendingPathComponent("xdgTestHelper")
54-
#endif
5547
}
5648

5749

Tests/Foundation/TestFileManager.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,6 +1235,9 @@ class TestFileManager : XCTestCase {
12351235
}
12361236

12371237
func test_fetchXDGPathsFromHelper() throws {
1238+
#if os(Windows)
1239+
throw XCTSkip("This test is disabled on Windows, needs investigation.")
1240+
#endif
12381241
let prefix = NSHomeDirectory() + "/_Foundation_Test_"
12391242

12401243
let configuration = """

Tests/Foundation/TestProcess.swift

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -363,11 +363,13 @@ class TestProcess : XCTestCase {
363363
process.arguments = ["--cat"]
364364
_ = try? process.run()
365365
XCTAssertTrue(process.isRunning)
366-
XCTAssertTrue(process.processIdentifier > 0)
366+
XCTAssertGreaterThan(process.processIdentifier, 0)
367367
process.terminate()
368368
process.waitUntilExit()
369369
XCTAssertFalse(process.isRunning)
370-
XCTAssertTrue(process.processIdentifier > 0)
370+
#if !os(Windows) // Since the processHandle is Closed on terminate, this cannot be determined.
371+
XCTAssertGreaterThan(process.processIdentifier, 0)
372+
#endif
371373
XCTAssertEqual(process.terminationReason, .uncaughtSignal)
372374
XCTAssertEqual(process.terminationStatus, SIGTERM)
373375
}
@@ -635,7 +637,7 @@ class TestProcess : XCTestCase {
635637
let one: String.Index = directory.index(zero, offsetBy: 1)
636638
XCTAssertTrue(directory[zero].isLetter)
637639
XCTAssertEqual(directory[one], ":")
638-
directory = "/" + String(directory.dropFirst(2))
640+
directory = String(directory.dropFirst(2))
639641
#endif
640642
XCTAssertEqual(URL(fileURLWithPath: directory).absoluteURL,
641643
URL(fileURLWithPath: "/").absoluteURL)

0 commit comments

Comments
 (0)