Disable tests which double-close file descriptors (#153)

A couple tests attempt to double-close a file descriptor. You cannot ever safely do this as even the act of spawning a process itself may have re-allocated the same file descriptor in the calling process - for example, the pidfd file descriptor, or other threads may have changed state. Disable these tests until we figure out what to do with them. It's possible the solution may involve #161, where Subprocess's own FileDescriptor type can have its own logic for refusing to double-close, and we validate the condition at that level rather than at the POSIX level.

This hasn't failed in CI so far due to the older Linux kernel in use.

Also moves some other code to using closeAfter to guarantee proper cleanup.

Author: jakepetroules

Tests/SubprocessTests/IntegrationTests.swift

@@ -973,17 +973,19 @@ extension SubprocessIntegrationTests {
             options: .create,
             permissions: [.ownerReadWrite, .groupReadWrite]
         )
-        let echoResult = try await _run(
-            setup,
-            input: .none,
-            output: .fileDescriptor(
-                outputFile,
-                closeAfterSpawningProcess: false
-            ),
-            error: .discarded
-        )
-        #expect(echoResult.terminationStatus.isSuccess)
-        try outputFile.close()
+        let echoResult = try await outputFile.closeAfter {
+            let echoResult = try await _run(
+                setup,
+                input: .none,
+                output: .fileDescriptor(
+                    outputFile,
+                    closeAfterSpawningProcess: false
+                ),
+                error: .discarded
+            )
+            #expect(echoResult.terminationStatus.isSuccess)
+            return echoResult
+        }
         let outputData: Data = try Data(
             contentsOf: URL(filePath: outputFilePath.string)
         )
@@ -994,7 +996,7 @@ extension SubprocessIntegrationTests {
         #expect(output == expected)
     }
 
-    @Test func testFileDescriptorOutputAutoClose() async throws {
+    @Test(.disabled("Cannot ever safely call unbalanced close() on the same fd")) func testFileDescriptorOutputAutoClose() async throws {
         #if os(Windows)
         let setup = TestSetup(
             executable: .name("cmd.exe"),
@@ -1254,17 +1256,19 @@ extension SubprocessIntegrationTests {
             options: .create,
             permissions: [.ownerReadWrite, .groupReadWrite]
         )
-        let echoResult = try await _run(
-            setup,
-            input: .none,
-            output: .discarded,
-            error: .fileDescriptor(
-                outputFile,
-                closeAfterSpawningProcess: false
+        let echoResult = try await outputFile.closeAfter {
+            let echoResult = try await _run(
+                setup,
+                input: .none,
+                output: .discarded,
+                error: .fileDescriptor(
+                    outputFile,
+                    closeAfterSpawningProcess: false
+                )
             )
-        )
-        #expect(echoResult.terminationStatus.isSuccess)
-        try outputFile.close()
+            #expect(echoResult.terminationStatus.isSuccess)
+            return echoResult
+        }
         let outputData: Data = try Data(
             contentsOf: URL(filePath: outputFilePath.string)
         )
@@ -1275,7 +1279,7 @@ extension SubprocessIntegrationTests {
         #expect(output == expected)
     }
 
-    @Test func testFileDescriptorErrorOutputAutoClose() async throws {
+    @Test(.disabled("Cannot ever safely call unbalanced close() on the same fd")) func testFileDescriptorErrorOutputAutoClose() async throws {
         #if os(Windows)
         let setup = TestSetup(
             executable: .name("cmd.exe"),
@@ -1338,20 +1342,22 @@ extension SubprocessIntegrationTests {
             options: .create,
             permissions: [.ownerReadWrite, .groupReadWrite]
         )
-        let echoResult = try await _run(
-            setup,
-            input: .none,
-            output: .fileDescriptor(
-                outputFile,
-                closeAfterSpawningProcess: false
-            ),
-            error: .fileDescriptor(
-                outputFile,
-                closeAfterSpawningProcess: false
+        let echoResult = try await outputFile.closeAfter {
+            let echoResult = try await _run(
+                setup,
+                input: .none,
+                output: .fileDescriptor(
+                    outputFile,
+                    closeAfterSpawningProcess: false
+                ),
+                error: .fileDescriptor(
+                    outputFile,
+                    closeAfterSpawningProcess: false
+                )
             )
-        )
-        #expect(echoResult.terminationStatus.isSuccess)
-        try outputFile.close()
+            #expect(echoResult.terminationStatus.isSuccess)
+            return echoResult
+        }
         let outputData: Data = try Data(
             contentsOf: URL(filePath: outputFilePath.string)
         )

Tests/SubprocessTests/ProcessMonitoringTests.swift

@@ -299,6 +299,12 @@ extension SubprocessProcessMonitoringTests {
         let testCount = 100
         var spawnedProcesses: [ProcessIdentifier] = []
 
+        defer {
+            for pid in spawnedProcesses {
+                pid.close()
+            }
+        }
+
         for _ in 0 ..< testCount {
             let config = self.immediateExitProcess(withExitCode: 0)
             let spawnResult = try config.spawn(
@@ -309,12 +315,6 @@ extension SubprocessProcessMonitoringTests {
             spawnedProcesses.append(spawnResult.execution.processIdentifier)
         }
 
-        defer {
-            for pid in spawnedProcesses {
-                pid.close()
-            }
-        }
-
         try await withThrowingTaskGroup { group in
             for pid in spawnedProcesses {
                 group.addTask {

Tests/SubprocessTests/UnixTests.swift

@@ -381,37 +381,36 @@ extension SubprocessUnixTests {
 
     @Test(.requiresBash) func testSubprocessDoesNotInheritRandomFileDescriptors() async throws {
         let pipe = try FileDescriptor.ssp_pipe()
-        defer {
-            try? pipe.readEnd.close()
-            try? pipe.writeEnd.close()
-        }
-        // Spawn bash and then attempt to write to the write end
-        let result = try await Subprocess.run(
-            .path("/bin/bash"),
-            arguments: [
-                "-c",
-                """
-                echo this string should be discarded >&\(pipe.writeEnd.rawValue);
-                echo wrote into \(pipe.writeEnd.rawValue), echo exit code $?;
-                """
-            ],
-            input: .none,
-            output: .string(limit: 64),
-            error: .discarded
-        )
-        try pipe.writeEnd.close()
-        #expect(result.terminationStatus.isSuccess)
-        // Make sure nothing is written to the pipe
-        var readBytes: [UInt8] = Array(repeating: 0, count: 1024)
-        let readCount = try readBytes.withUnsafeMutableBytes { ptr in
-            return try FileDescriptor(rawValue: pipe.readEnd.rawValue)
-                .read(into: ptr, retryOnInterrupt: true)
+        try await pipe.readEnd.closeAfter {
+            let result = try await pipe.writeEnd.closeAfter {
+                // Spawn bash and then attempt to write to the write end
+                try await Subprocess.run(
+                    .path("/bin/bash"),
+                    arguments: [
+                        "-c",
+                        """
+                        echo this string should be discarded >&\(pipe.writeEnd.rawValue);
+                        echo wrote into \(pipe.writeEnd.rawValue), echo exit code $?;
+                        """
+                    ],
+                    input: .none,
+                    output: .string(limit: 64),
+                    error: .discarded
+                )
+            }
+            #expect(result.terminationStatus.isSuccess)
+            // Make sure nothing is written to the pipe
+            var readBytes: [UInt8] = Array(repeating: 0, count: 1024)
+            let readCount = try readBytes.withUnsafeMutableBytes { ptr in
+                return try FileDescriptor(rawValue: pipe.readEnd.rawValue)
+                    .read(into: ptr, retryOnInterrupt: true)
+            }
+            #expect(readCount == 0)
+            #expect(
+                result.standardOutput?.trimmingNewLineAndQuotes() ==
+                "wrote into \(pipe.writeEnd.rawValue), echo exit code 1"
+            )
         }
-        #expect(readCount == 0)
-        #expect(
-            result.standardOutput?.trimmingNewLineAndQuotes() ==
-            "wrote into \(pipe.writeEnd.rawValue), echo exit code 1"
-        )
     }
 }