Fix file descriptor leak (#315)

Author: kkebo

Sources/WASI/Platform/Directory.swift

@@ -99,7 +99,9 @@ extension DirEntry: WASIDir, FdWASIEntry {
     }
 
     func removeFile(atPath path: String) throws {
-        let (dir, basename) = try SandboxPrimitives.openParent(start: fd, path: path)
+        let result = try SandboxPrimitives.openParent(start: fd, path: path)
+        let dir = result.parentFd
+        let basename = result.basename
         try WASIAbi.Errno.translatingPlatformErrno {
             try dir.remove(at: FilePath(basename), options: [])
         }
@@ -109,17 +111,21 @@ extension DirEntry: WASIDir, FdWASIEntry {
         #if os(Windows)
             throw WASIAbi.Errno.ENOSYS
         #else
-            let (dir, basename) = try SandboxPrimitives.openParent(start: fd, path: path)
+            let result = try SandboxPrimitives.openParent(start: fd, path: path)
+            let dir = result.parentFd
+            let basename = result.basename
             try WASIAbi.Errno.translatingPlatformErrno {
                 try dir.remove(at: FilePath(basename), options: .removeDirectory)
             }
         #endif
     }
 
     func symlink(from sourcePath: String, to destPath: String) throws {
-        let (destDir, destBasename) = try SandboxPrimitives.openParent(
+        let result = try SandboxPrimitives.openParent(
             start: fd, path: destPath
         )
+        let destDir = result.parentFd
+        let destBasename = result.basename
         try WASIAbi.Errno.translatingPlatformErrno {
             try destDir.createSymlink(original: FilePath(sourcePath), link: FilePath(destBasename))
         }
@@ -142,12 +148,16 @@ extension DirEntry: WASIDir, FdWASIEntry {
             let oldPath = SandboxPrimitives.stripDirSuffix(sourcePath)
             let newPath = SandboxPrimitives.stripDirSuffix(destPath)
 
-            let (sourceDir, sourceBasename) = try SandboxPrimitives.openParent(
+            let sourceResult = try SandboxPrimitives.openParent(
                 start: fd, path: oldPath
             )
-            let (destDir, destBasename) = try SandboxPrimitives.openParent(
+            let sourceDir = sourceResult.parentFd
+            let sourceBasename = sourceResult.basename
+            let destResult = try SandboxPrimitives.openParent(
                 start: newDir.fd, path: newPath
             )
+            let destDir = destResult.parentFd
+            let destBasename = destResult.basename
 
             // Re-append a slash if the original path had one
             let finalSourceBasename = oldHasTrailingSlash ? sourceBasename + "/" : sourceBasename
@@ -204,7 +214,9 @@ extension DirEntry: WASIDir, FdWASIEntry {
     }
 
     func createDirectory(atPath path: String) throws {
-        let (dir, basename) = try SandboxPrimitives.openParent(start: fd, path: path)
+        let result = try SandboxPrimitives.openParent(start: fd, path: path)
+        let dir = result.parentFd
+        let basename = result.basename
         try WASIAbi.Errno.translatingPlatformErrno {
             try dir.createDirectory(at: FilePath(basename), permissions: .ownerReadWriteExecute)
         }
@@ -218,7 +230,9 @@ extension DirEntry: WASIDir, FdWASIEntry {
             if !symlinkFollow {
                 options.insert(.noFollow)
             }
-            let (dir, basename) = try SandboxPrimitives.openParent(start: fd, path: path)
+            let result = try SandboxPrimitives.openParent(start: fd, path: path)
+            let dir = result.parentFd
+            let basename = result.basename
             let attributes = try basename.withCString { cBasename in
                 try WASIAbi.Errno.translatingPlatformErrno {
                     try dir.attributes(at: cBasename, options: options)

Sources/WASI/Platform/SandboxPrimitives/OpenParent.swift

@@ -47,27 +47,50 @@ extension SandboxPrimitives {
         return path.hasSuffix("/")
     }
 
-    static func openParent(start: FileDescriptor, path: String) throws -> (FileDescriptor, String) {
+    static func openParent(start: FileDescriptor, path: String) throws -> SplitPath {
         guard let (dirName, basename) = splitParent(path: path) else {
             throw WASIAbi.Errno.ENOENT
         }
 
-        let dirFd: FileDescriptor
+        let splitPath: SplitPath
         if !dirName.isEmpty {
             let options: FileDescriptor.OpenOptions
             #if os(Windows)
                 options = []
             #else
                 options = .directory
             #endif
-            dirFd = try openAt(
-                start: start, path: dirName,
-                mode: .readOnly, options: options,
-                permissions: []
+            splitPath = try .init(
+                parentFd: openAt(
+                    start: start, path: dirName,
+                    mode: .readOnly, options: options,
+                    permissions: []
+                ),
+                basename: basename.string,
+                isTemporary: true
             )
         } else {
-            dirFd = start
+            splitPath = .init(
+                parentFd: start,
+                basename: basename.string,
+                isTemporary: false
+            )
+        }
+        return splitPath
+    }
+}
+
+/// The return value of `SandboxPrimitives.openParent`.
+///
+/// If `isTemporary` is `true`, the file descriptor will be closed when the instance is destroyed.
+struct SplitPath: ~Copyable {
+    var parentFd: FileDescriptor
+    var basename: String
+    var isTemporary: Bool
+
+    deinit {
+        if isTemporary {
+            try? parentFd.close()
         }
-        return (dirFd, basename.string)
     }
 }

Sources/WASI/Platform/SandboxPrimitives/Readlink.swift

@@ -29,12 +29,9 @@ extension SandboxPrimitives {
             let initialBufferCapacity = 256
             let maxBufferCapacity = max(initialBufferCapacity, Int(PATH_MAX))
 
-            let (dir, basename) = try openParent(start: start, path: path)
-            defer {
-                if dir.rawValue != start.rawValue {
-                    try? dir.close()
-                }
-            }
+            let result = try openParent(start: start, path: path)
+            let dir = result.parentFd
+            let basename = result.basename
 
             return try basename.withCString { cBasename in
                 var capacity = min(initialBufferCapacity, maxBufferCapacity)

Tests/WASITests/WASITests.swift

@@ -1347,4 +1347,62 @@ struct WASITests {
         }
     #endif
 
+    #if os(macOS) || os(Linux)
+        // https://github.com/swiftwasm/WasmKit/issues/275
+        @Test
+        func fileDescriptorLeakTest() throws {
+            let t = try TestSupport.TemporaryDirectory()
+
+            try t.createDir(at: "dir1")
+            try t.createDir(at: "dir1/foo")
+            try t.createDir(at: "dir2")
+            try t.createDir(at: "dir2/foo")
+
+            let wasi = try WASIBridgeToHost(
+                fileSystem: .host().withPreopens([
+                    .init(guestPath: "/dir1", hostPath: t.url.appending(component: "dir1").path),
+                    .init(guestPath: "/dir2", hostPath: t.url.appending(component: "dir2").path),
+                ])
+            ).underlying
+            let dir1Fd: WASIAbi.Fd = 3
+            let dir2Fd: WASIAbi.Fd = 4
+
+            let memory = TestSupport.TestGuestMemory()
+            let buffer = UnsafeGuestBufferPointer<UInt8>(
+                baseAddress: UnsafeGuestPointer<UInt8>(memorySpace: memory, offset: 0),
+                count: 4096
+            )
+            let writeData = Array("hello".utf8)
+            let writeVecs = memory.writeIOVecs([writeData])
+
+            for _ in 0..<1000 {
+                let fd = try wasi.path_open(
+                    dirFd: dir1Fd,
+                    dirFlags: [],
+                    path: "foo/bar",
+                    oflags: [.CREAT],
+                    fsRightsBase: [.FD_WRITE],
+                    fsRightsInheriting: [],
+                    fdflags: []
+                )
+                #expect(try wasi.fd_write(fileDescriptor: fd, ioVectors: writeVecs) > 0)
+                try wasi.fd_close(fd: fd)
+
+                try wasi.path_rename(oldFd: dir1Fd, oldPath: "foo/bar", newFd: dir2Fd, newPath: "foo/baz")
+
+                try wasi.path_symlink(oldPath: "baz", dirFd: dir2Fd, newPath: "foo/quux")
+                let stat = try wasi.path_filestat_get(dirFd: dir2Fd, flags: .SYMLINK_FOLLOW, path: "foo/quux")
+                #expect(stat.size == 5)  // hello
+                let count = try Int(wasi.path_readlink(fd: dir2Fd, path: "foo/quux", buffer: buffer))
+                buffer.withHostPointer { ptr in
+                    #expect(String(decoding: ptr[..<count], as: UTF8.self) == "baz")
+                }
+                try wasi.path_unlink_file(dirFd: dir2Fd, path: "foo/baz")
+                try wasi.path_unlink_file(dirFd: dir2Fd, path: "foo/quux")
+
+                try wasi.path_create_directory(dirFd: dir1Fd, path: "foo/bar")
+                try wasi.path_remove_directory(dirFd: dir1Fd, path: "foo/bar")
+            }
+        }
+    #endif
 }