SR-13822: FileHandle closeOnDealloc is not respected

Author: spevans

Sources/Foundation/FileHandle.swift

@@ -477,7 +477,7 @@ open class FileHandle : NSObject {
         // - deinit tries to .sync { … } to serialize the work on the handle queue, _which we're already on_
         // - deadlock! DispatchQueue's deadlock detection triggers and crashes us.
         // since all operations on the handle queue retain the handle during use, if the handle is being deinited, then there are no more operations on the queue, so this is serial with respect to them anyway. Just close the handle immediately.
-        try? _immediatelyClose()
+        try? _immediatelyClose(closeFd: _closeOnDealloc)
     }
 
     // MARK: -
@@ -620,7 +620,7 @@ open class FileHandle : NSObject {
         }
     }
 
-    private func _immediatelyClose() throws {
+    private func _immediatelyClose(closeFd: Bool = true) throws {
         guard self != FileHandle._nulldeviceFileHandle else { return }
         guard _isPlatformHandleValid else { return }
 
@@ -632,18 +632,20 @@ open class FileHandle : NSObject {
         writabilitySource = nil
         readabilitySource = nil
         privateAsyncVariablesLock.unlock()
-        
-        #if os(Windows)
-        guard CloseHandle(self._handle) else {
-            throw _NSErrorWithWindowsError(GetLastError(), reading: true)
-        }
-        self._handle = INVALID_HANDLE_VALUE
-        #else
-        guard _close(_fd) >= 0 else {
-            throw _NSErrorWithErrno(errno, reading: true)
+
+        if closeFd {
+#if os(Windows)
+            guard CloseHandle(self._handle) else {
+                throw _NSErrorWithWindowsError(GetLastError(), reading: true)
+            }
+            self._handle = INVALID_HANDLE_VALUE
+#else
+            guard _close(_fd) >= 0 else {
+                throw _NSErrorWithErrno(errno, reading: true)
+            }
+            _fd = -1
+#endif
         }
-        _fd = -1
-        #endif
     }
 
     // MARK: -

Tests/Foundation/Tests/TestFileHandle.swift

@@ -579,7 +579,40 @@ class TestFileHandle : XCTestCase {
             XCTAssertTrue(notificationReceived, "Notification should be sent")
         }
     }
-    
+
+#if NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT
+    func test_closeOnDealloc() throws {
+        try withTemporaryDirectory() { (url, path) in
+            let data = try XCTUnwrap("hello".data(using: .utf8))
+
+            // closeOnDealloc: true, 2nd write should throw.
+            do {
+                let fileUrl = url.appendingPathComponent("testfile")
+                let fd = try FileHandle._openFileDescriptorForURL(fileUrl, flags: O_CREAT | O_RDWR, reading: false)
+                do {
+                    let fh = FileHandle(fileDescriptor: fd, closeOnDealloc: true)
+                    XCTAssertNoThrow(try fh.write(contentsOf: data))
+                }
+                let fh2 = FileHandle(fileDescriptor: fd, closeOnDealloc: true)
+                XCTAssertThrowsError(try fh2.write(contentsOf: data))
+            }
+
+            // closeOnDealloc: false, 2nd write should succeed.
+            do {
+                let fileUrl = url.appendingPathComponent("testfile2")
+                let fd = try FileHandle._openFileDescriptorForURL(fileUrl, flags: O_CREAT | O_RDWR, reading: false)
+                do {
+                    let fh = FileHandle(fileDescriptor: fd, closeOnDealloc: false)
+                    XCTAssertNoThrow(try fh.write(contentsOf: data))
+                }
+                // Close the file handle after this write, dont leave it open after this test.
+                let fh2 = FileHandle(fileDescriptor: fd, closeOnDealloc: true)
+                XCTAssertNoThrow(try fh2.write(contentsOf: data))
+            }
+        }
+    }
+#endif
+
     static var allTests : [(String, (TestFileHandle) -> () throws -> ())] {
         var tests: [(String, (TestFileHandle) -> () throws -> ())] = [
             ("testReadUpToCount", testReadUpToCount),
@@ -607,6 +640,7 @@ class TestFileHandle : XCTestCase {
             ("test_nullDevice", test_nullDevice),
             ("testHandleCreationAndCleanup", testHandleCreationAndCleanup),
             ("testOffset", testOffset),
+            ("test_closeOnDealloc", test_closeOnDealloc),
         ])
 #endif