Merge pull request #5270 from daveinglis/fix_handle_leak

Fix process handle leak on Windows

Author: iCharlesHu

Package.swift

@@ -356,7 +356,7 @@ let package = Package(
                 "FoundationNetworking",
                 "XCTest",
                 "Testing",
-                .target(name: "xdgTestHelper", condition: .when(platforms: [.linux, .android]))
+                .target(name: "xdgTestHelper", condition: .when(platforms: [.linux, .android, .windows]))
             ],
             resources: [
                 .copy("Foundation/Resources")

Sources/Foundation/Process.swift

@@ -690,6 +690,7 @@ open class Process: NSObject, @unchecked Sendable {
         if !CloseHandle(piProcessInfo.hThread) {
           throw _NSErrorWithWindowsError(GetLastError(), reading: false)
         }
+        self.processIdentifier = Int32(GetProcessId(self.processHandle))
 
         if let pipe = standardInput as? Pipe {
           pipe.fileHandleForReading.closeFile()
@@ -1102,26 +1103,11 @@ open class Process: NSObject, @unchecked Sendable {
     // status
 #if os(Windows)
     open private(set) var processHandle: HANDLE = INVALID_HANDLE_VALUE
-    open var processIdentifier: Int32 {
-      guard processHandle != INVALID_HANDLE_VALUE else {
-          return 0
-      }
-      return Int32(GetProcessId(processHandle))
-    }
-    open private(set) var isRunning: Bool = false
-
-    private var hasStarted: Bool {
-      return processHandle != INVALID_HANDLE_VALUE
-    }
-    private var hasFinished: Bool {
-      return hasStarted && !isRunning
-    }
-#else
+#endif
     open private(set) var processIdentifier: Int32 = 0
     open private(set) var isRunning: Bool = false
     private var hasStarted: Bool { return processIdentifier > 0 }
     private var hasFinished: Bool { return !isRunning && processIdentifier > 0 }
-#endif
 
     private var _terminationStatus: Int32 = 0
     public var terminationStatus: Int32 {
@@ -1200,6 +1186,17 @@ open class Process: NSObject, @unchecked Sendable {
         if let handler = self.terminationHandler {
             let thread: Thread = Thread { handler(self) }
             thread.start()
+            closeHandler()
+        } else {
+            closeHandler()
+        }
+
+        // This closeHandler is called as late as possible 
+        // so the processHandle (on Windows) is valid for as long as possible. 
+        func closeHandler() {
+#if os(Windows)
+            CloseHandle(self.processHandle)
+#endif
         }
     }
 }

Tests/Foundation/TestBundle.swift

@@ -43,15 +43,7 @@ internal func testBundleName() -> String {
 }
 
 internal func xdgTestHelperURL() throws -> URL {
-    #if os(Windows)
-    // 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
-    // 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
-    // SwiftPM also cannot support depending on this executable (to ensure it is built) without also linking its objects into the test runner
-    // For those reasons, using the xdgTestHelper on Windows is currently unsupported and tests that rely on it must be skipped
-    throw XCTSkip("xdgTestHelper is not supported during testing on Windows (test executables are not supported by SwiftPM on Windows)")
-    #else
     testBundle().bundleURL.deletingLastPathComponent().appendingPathComponent("xdgTestHelper")
-    #endif
 }
 
 

Tests/Foundation/TestFileManager.swift

@@ -1235,6 +1235,9 @@ class TestFileManager : XCTestCase {
     }
 
     func test_fetchXDGPathsFromHelper() throws {
+        #if os(Windows)
+        throw XCTSkip("This test is disabled on Windows, needs investigation.")
+        #endif
         let prefix = NSHomeDirectory() + "/_Foundation_Test_"
 
         let configuration = """

Tests/Foundation/TestProcess.swift

@@ -363,11 +363,11 @@ class TestProcess : XCTestCase {
         process.arguments = ["--cat"]
         _ = try? process.run()
         XCTAssertTrue(process.isRunning)
-        XCTAssertTrue(process.processIdentifier > 0)
+        XCTAssertGreaterThan(process.processIdentifier, 0)
         process.terminate()
         process.waitUntilExit()
         XCTAssertFalse(process.isRunning)
-        XCTAssertTrue(process.processIdentifier > 0)
+        XCTAssertGreaterThan(process.processIdentifier, 0)
         XCTAssertEqual(process.terminationReason, .uncaughtSignal)
         XCTAssertEqual(process.terminationStatus, SIGTERM)
     }
@@ -635,7 +635,7 @@ class TestProcess : XCTestCase {
             let one: String.Index = directory.index(zero, offsetBy: 1)
             XCTAssertTrue(directory[zero].isLetter)
             XCTAssertEqual(directory[one], ":")
-            directory = "/" + String(directory.dropFirst(2))
+            directory = String(directory.dropFirst(2))
 #endif
             XCTAssertEqual(URL(fileURLWithPath: directory).absoluteURL,
                            URL(fileURLWithPath: "/").absoluteURL)