Skip to content

Commit d6447a6

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 d6447a6

File tree

5 files changed

+18
-28
lines changed

5 files changed

+18
-28
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: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,7 @@ open class Process: NSObject, @unchecked Sendable {
690690
if !CloseHandle(piProcessInfo.hThread) {
691691
throw _NSErrorWithWindowsError(GetLastError(), reading: false)
692692
}
693+
self.processIdentifier = Int32(GetProcessId(self.processHandle))
693694

694695
if let pipe = standardInput as? Pipe {
695696
pipe.fileHandleForReading.closeFile()
@@ -1102,26 +1103,11 @@ open class Process: NSObject, @unchecked Sendable {
11021103
// status
11031104
#if os(Windows)
11041105
open private(set) var processHandle: HANDLE = INVALID_HANDLE_VALUE
1105-
open var processIdentifier: Int32 {
1106-
guard processHandle != INVALID_HANDLE_VALUE else {
1107-
return 0
1108-
}
1109-
return Int32(GetProcessId(processHandle))
1110-
}
1111-
open private(set) var isRunning: Bool = false
1112-
1113-
private var hasStarted: Bool {
1114-
return processHandle != INVALID_HANDLE_VALUE
1115-
}
1116-
private var hasFinished: Bool {
1117-
return hasStarted && !isRunning
1118-
}
1119-
#else
1106+
#endif
11201107
open private(set) var processIdentifier: Int32 = 0
11211108
open private(set) var isRunning: Bool = false
11221109
private var hasStarted: Bool { return processIdentifier > 0 }
11231110
private var hasFinished: Bool { return !isRunning && processIdentifier > 0 }
1124-
#endif
11251111

11261112
private var _terminationStatus: Int32 = 0
11271113
public var terminationStatus: Int32 {
@@ -1200,6 +1186,15 @@ open class Process: NSObject, @unchecked Sendable {
12001186
if let handler = self.terminationHandler {
12011187
let thread: Thread = Thread { handler(self) }
12021188
thread.start()
1189+
closeHandler()
1190+
} else {
1191+
closeHandler()
1192+
}
1193+
1194+
func closeHandler() {
1195+
#if os(Windows)
1196+
CloseHandle(self.processHandle)
1197+
#endif
12031198
}
12041199
}
12051200
}

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: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -363,11 +363,11 @@ 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+
XCTAssertGreaterThan(process.processIdentifier, 0)
371371
XCTAssertEqual(process.terminationReason, .uncaughtSignal)
372372
XCTAssertEqual(process.terminationStatus, SIGTERM)
373373
}
@@ -635,7 +635,7 @@ class TestProcess : XCTestCase {
635635
let one: String.Index = directory.index(zero, offsetBy: 1)
636636
XCTAssertTrue(directory[zero].isLetter)
637637
XCTAssertEqual(directory[one], ":")
638-
directory = "/" + String(directory.dropFirst(2))
638+
directory = String(directory.dropFirst(2))
639639
#endif
640640
XCTAssertEqual(URL(fileURLWithPath: directory).absoluteURL,
641641
URL(fileURLWithPath: "/").absoluteURL)

0 commit comments

Comments
 (0)