Skip to content

Commit 73cc331

Browse files
authored
Fix fd handling in WASIp1 implementation (#327)
`PathResolution.resolve()` returned the input fd unchanged for "." paths, causing fd table entries to alias the same host fd. Closing one invalidated the other, which errored with EBADF. Multiple related fixes are included to clean up the general situation with FD handling in WASIp1 implementation: - Fix fd aliasing in `PathResolution.resolve()`: When the path resolves without opening a new fd (e.g. "."), dup via `openat(fd, ".")` so callers always get an independently closeable fd. - Fix `SplitPath` premature deinit: Use `borrowing func withFields` to keep `SplitPath: ~Copyable` alive during operations that use its parent fd. - Add `WASIBridgeToHost.runAndClose`: Scoped API that ensures `close()` is called, replacing the previous `deinit`-based cleanup. - Add `isBorrowed` to `WASIEntry`: Stdio entries are marked as borrowed so `closeAll()` never closes process stdin/stdout/stderr (previously broken when `fd_renumber` moved stdio to different WASI fd positions). - Fix `directoryEntry(fd:)` error codes: Return `EBADF` for missing fds, `ENOTDIR` for non-directory fds (was `ENOTDIR` for both). I ran tests locally 10 times in a row and on CI 5 times in a row on this PR, no failures, I think this is sufficient to say that flakiness is resolved. Resolves #334.
1 parent d85cd17 commit 73cc331

20 files changed

Lines changed: 1146 additions & 842 deletions

File tree

Benchmarks/Benchmarks/MacroPlugin/MacroPlugin.swift

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ let benchmarks: @Sendable () -> () = {
151151
let pluginToHost: FileDescriptor
152152
let pump: Function
153153
let expandMessage: String
154+
let bridge: WASIBridgeToHost
154155

155156
init(filePath: FilePath, expandMessage: String) throws {
156157
let engine = Engine()
@@ -164,16 +165,22 @@ let benchmarks: @Sendable () -> () = {
164165
stdout: pluginToHostPipes.writeEnd,
165166
stderr: .standardError
166167
)
167-
var imports = Imports()
168-
bridge.link(to: &imports, store: store)
169-
let instance = try module.instantiate(store: store, imports: imports)
170-
try instance.exports[function: "_start"]!()
171-
let pump = instance.exports[function: "swift_wasm_macro_v1_pump"]!
172-
173-
self.hostToPlugin = hostToPluginPipes.writeEnd
174-
self.pluginToHost = pluginToHostPipes.readEnd
175-
self.pump = pump
176-
self.expandMessage = expandMessage
168+
do {
169+
var imports = Imports()
170+
bridge.link(to: &imports, store: store)
171+
let instance = try module.instantiate(store: store, imports: imports)
172+
try instance.exports[function: "_start"]!()
173+
let pump = instance.exports[function: "swift_wasm_macro_v1_pump"]!
174+
175+
self.hostToPlugin = hostToPluginPipes.writeEnd
176+
self.pluginToHost = pluginToHostPipes.readEnd
177+
self.pump = pump
178+
self.expandMessage = expandMessage
179+
self.bridge = bridge
180+
} catch {
181+
try bridge.close()
182+
throw error
183+
}
177184
}
178185

179186
func writeMessage(_ message: String) throws {
@@ -204,6 +211,10 @@ let benchmarks: @Sendable () -> () = {
204211
try pump()
205212
_ = try readMessage()
206213
}
214+
215+
func close() throws {
216+
try bridge.close()
217+
}
207218
}
208219

209220
guard let expandMessage = expandMessages[file] else {
@@ -214,10 +225,12 @@ let benchmarks: @Sendable () -> () = {
214225
let setup = try Setup(filePath: macrosDir.appending(file), expandMessage: expandMessage)
215226
try setup.writeMessage(handshakeMessage)
216227
try setup.tick()
228+
try setup.close()
217229
}
218230

219231
Benchmark("Expand \(file)") { benchmark, setup in
220232
try setup.tick()
233+
try setup.close()
221234
} setup: { () -> Setup in
222235
let setup = try Setup(
223236
filePath: macrosDir.appending(file),

Benchmarks/Benchmarks/WishYouWereFast/WishYouWereFast.swift

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,12 @@ let benchmarks: @Sendable () -> () = {
3131
filePath: FilePath(wishYouWereFast.appendingPathComponent(file).path)
3232
)
3333
let wasi = try WASIBridgeToHost(fileSystem: .host().withStdio(stdout: devNull, stderr: devNull))
34-
var imports = Imports()
35-
wasi.link(to: &imports, store: store)
36-
let instance = try module.instantiate(store: store, imports: imports)
37-
_ = try wasi.start(instance)
34+
_ = try wasi.runAndClose { wasi in
35+
var imports = Imports()
36+
wasi.link(to: &imports, store: store)
37+
let instance = try module.instantiate(store: store, imports: imports)
38+
return try wasi.start(instance)
39+
}
3840
}
3941
}
4042
}

Examples/Sources/WASI-Hello/Hello.swift

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,17 @@ struct Example {
1212

1313
// Create a WASI instance forwarding to the host environment.
1414
let wasi = try WASIBridgeToHost()
15-
// Create engine and store
16-
let engine = Engine()
17-
let store = Store(engine: engine)
18-
// Instantiate a parsed module importing WASI
19-
var imports = Imports()
20-
wasi.link(to: &imports, store: store)
21-
let instance = try module.instantiate(store: store, imports: imports)
22-
23-
// Start the WASI command-line application.
24-
let exitCode = try wasi.start(instance)
15+
// Start the WASI command-line application and close the bridge.
16+
let exitCode = try wasi.runAndClose { wasi in
17+
// Create engine and store
18+
let engine = Engine()
19+
let store = Store(engine: engine)
20+
// Instantiate a parsed module importing WASI
21+
var imports = Imports()
22+
wasi.link(to: &imports, store: store)
23+
let instance = try module.instantiate(store: store, imports: imports)
24+
return try wasi.start(instance)
25+
}
2526
// Exit the Swift program with the WASI exit code.
2627
exit(Int32(exitCode))
2728
}

Sources/CLICommands/DebuggerServer.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@
113113
await shutDownStream.first { _ in true }
114114
cancellableGroup.cancelAll()
115115
}
116+
try await debuggerHandler.close()
116117
}
117118
}
118119
}

Sources/CLICommands/Run.swift

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -298,12 +298,14 @@ package struct Run: AsyncParsableCommand {
298298
let wasi = try WASIBridgeToHost(args: [path] + arguments, environment: environment, preopens: preopens)
299299
let engine = Engine(configuration: deriveRuntimeConfiguration(), interceptor: interceptor)
300300
let store = Store(engine: engine)
301-
var imports = Imports()
302-
wasi.link(to: &imports, store: store)
303-
let moduleInstance = try module.instantiate(store: store, imports: imports)
304301
return {
305-
let exitCode = try wasi.start(moduleInstance)
306-
throw ExitCode(Int32(exitCode))
302+
try wasi.runAndClose { wasi in
303+
var imports = Imports()
304+
wasi.link(to: &imports, store: store)
305+
let moduleInstance = try module.instantiate(store: store, imports: imports)
306+
let exitCode = try wasi.start(moduleInstance)
307+
throw ExitCode(Int32(exitCode))
308+
}
307309
}
308310
}
309311

Sources/WASI/FileSystem.swift

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ struct FileAccessMode: OptionSet {
88
}
99

1010
protocol WASIEntry {
11+
/// Whether this entry wraps a borrowed file descriptor that should not be
12+
/// closed when the WASI instance is torn down (e.g. process stdio).
13+
var isBorrowed: Bool { get }
1114
func attributes() throws -> WASIAbi.Filestat
1215
func fileType() throws -> WASIAbi.FileType
1316
func status() throws -> WASIAbi.Fdflags
@@ -21,6 +24,10 @@ protocol WASIEntry {
2124
func close() throws
2225
}
2326

27+
extension WASIEntry {
28+
var isBorrowed: Bool { false }
29+
}
30+
2431
protocol WASIFile: WASIEntry {
2532
func fdStat() throws -> WASIAbi.FdStat
2633
func setFdStatFlags(_ flags: WASIAbi.Fdflags) throws
@@ -129,6 +136,22 @@ struct FdTable {
129136
return fd
130137
}
131138
}
139+
140+
/// Closes all owned file descriptors, skipping borrowed ones (e.g. stdio).
141+
mutating func closeAll() throws {
142+
var firstError: (any Error)?
143+
for (_, entry) in map {
144+
let wasiEntry = entry.asEntry()
145+
guard !wasiEntry.isBorrowed else { continue }
146+
do {
147+
try wasiEntry.close()
148+
} catch {
149+
if firstError == nil { firstError = error }
150+
}
151+
}
152+
map = map.filter { $0.value.asEntry().isBorrowed }
153+
if let firstError { throw firstError }
154+
}
132155
}
133156

134157
/// Content of a file that can be retrieved from the file system.

Sources/WASI/Platform/Directory.swift

Lines changed: 38 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ extension DirEntry: WASIDir, FdWASIEntry {
9090
symlinkFollow: symlinkFollow, path: path,
9191
oflags: [], accessMode: .write, fdflags: []
9292
)
93+
defer { try? fd.close() }
9394
let (access, modification) = try WASIAbi.Timestamp.platformTimeSpec(
9495
atim: atim, mtim: mtim, fstFlags: fstFlags
9596
)
@@ -100,10 +101,10 @@ extension DirEntry: WASIDir, FdWASIEntry {
100101

101102
func removeFile(atPath path: String) throws {
102103
let result = try SandboxPrimitives.openParent(start: fd, path: path)
103-
let dir = result.parentFd
104-
let basename = result.basename
105-
try WASIAbi.Errno.translatingPlatformErrno {
106-
try dir.remove(at: FilePath(basename), options: [])
104+
try result.withFields { dir, basename in
105+
try WASIAbi.Errno.translatingPlatformErrno {
106+
try dir.remove(at: FilePath(basename), options: [])
107+
}
107108
}
108109
}
109110

@@ -113,10 +114,10 @@ extension DirEntry: WASIDir, FdWASIEntry {
113114
#else
114115
let path = SandboxPrimitives.stripDirSuffix(path)
115116
let result = try SandboxPrimitives.openParent(start: fd, path: path)
116-
let dir = result.parentFd
117-
let basename = result.basename
118-
try WASIAbi.Errno.translatingPlatformErrno {
119-
try dir.remove(at: FilePath(basename), options: .removeDirectory)
117+
try result.withFields { dir, basename in
118+
try WASIAbi.Errno.translatingPlatformErrno {
119+
try dir.remove(at: FilePath(basename), options: .removeDirectory)
120+
}
120121
}
121122
#endif
122123
}
@@ -125,10 +126,10 @@ extension DirEntry: WASIDir, FdWASIEntry {
125126
let result = try SandboxPrimitives.openParent(
126127
start: fd, path: destPath
127128
)
128-
let destDir = result.parentFd
129-
let destBasename = result.basename
130-
try WASIAbi.Errno.translatingPlatformErrno {
131-
try destDir.createSymlink(original: FilePath(sourcePath), link: FilePath(destBasename))
129+
try result.withFields { destDir, destBasename in
130+
try WASIAbi.Errno.translatingPlatformErrno {
131+
try destDir.createSymlink(original: FilePath(sourcePath), link: FilePath(destBasename))
132+
}
132133
}
133134
}
134135

@@ -152,24 +153,23 @@ extension DirEntry: WASIDir, FdWASIEntry {
152153
let sourceResult = try SandboxPrimitives.openParent(
153154
start: fd, path: oldPath
154155
)
155-
let sourceDir = sourceResult.parentFd
156-
let sourceBasename = sourceResult.basename
157156
let destResult = try SandboxPrimitives.openParent(
158157
start: newDir.fd, path: newPath
159158
)
160-
let destDir = destResult.parentFd
161-
let destBasename = destResult.basename
162-
163-
// Re-append a slash if the original path had one
164-
let finalSourceBasename = oldHasTrailingSlash ? sourceBasename + "/" : sourceBasename
165-
let finalDestBasename = newHasTrailingSlash ? destBasename + "/" : destBasename
166-
167-
try WASIAbi.Errno.translatingPlatformErrno {
168-
try sourceDir.rename(
169-
at: FilePath(finalSourceBasename),
170-
to: destDir,
171-
at: FilePath(finalDestBasename)
172-
)
159+
try sourceResult.withFields { sourceDir, sourceBasename in
160+
try destResult.withFields { destDir, destBasename in
161+
// Re-append a slash if the original path had one
162+
let finalSourceBasename = oldHasTrailingSlash ? sourceBasename + "/" : sourceBasename
163+
let finalDestBasename = newHasTrailingSlash ? destBasename + "/" : destBasename
164+
165+
try WASIAbi.Errno.translatingPlatformErrno {
166+
try sourceDir.rename(
167+
at: FilePath(finalSourceBasename),
168+
to: destDir,
169+
at: FilePath(finalDestBasename)
170+
)
171+
}
172+
}
173173
}
174174
#endif
175175
}
@@ -216,10 +216,10 @@ extension DirEntry: WASIDir, FdWASIEntry {
216216

217217
func createDirectory(atPath path: String) throws {
218218
let result = try SandboxPrimitives.openParent(start: fd, path: path)
219-
let dir = result.parentFd
220-
let basename = result.basename
221-
try WASIAbi.Errno.translatingPlatformErrno {
222-
try dir.createDirectory(at: FilePath(basename), permissions: .ownerReadWriteExecute)
219+
try result.withFields { dir, basename in
220+
try WASIAbi.Errno.translatingPlatformErrno {
221+
try dir.createDirectory(at: FilePath(basename), permissions: .ownerReadWriteExecute)
222+
}
223223
}
224224
}
225225

@@ -232,15 +232,15 @@ extension DirEntry: WASIDir, FdWASIEntry {
232232
options.insert(.noFollow)
233233
}
234234
let result = try SandboxPrimitives.openParent(start: fd, path: path)
235-
let dir = result.parentFd
236-
let basename = result.basename
237-
let attributes = try basename.withCString { cBasename in
238-
try WASIAbi.Errno.translatingPlatformErrno {
239-
try dir.attributes(at: cBasename, options: options)
235+
return try result.withFields { dir, basename in
236+
let attributes = try basename.withCString { cBasename in
237+
try WASIAbi.Errno.translatingPlatformErrno {
238+
try dir.attributes(at: cBasename, options: options)
239+
}
240240
}
241-
}
242241

243-
return WASIAbi.Filestat(stat: attributes)
242+
return WASIAbi.Filestat(stat: attributes)
243+
}
244244
#endif
245245
}
246246
}

Sources/WASI/Platform/File.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ struct StdioFileEntry: FdWASIFile {
106106
let fd: FileDescriptor
107107
let accessMode: FileAccessMode
108108

109+
let isBorrowed: Bool = true
110+
109111
func attributes() throws -> WASIAbi.Filestat {
110112
return WASIAbi.Filestat(
111113
dev: 0, ino: 0, filetype: .CHARACTER_DEVICE,

Sources/WASI/Platform/SandboxPrimitives/Open.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,15 @@ struct PathResolution {
227227
case .regular: try regular(component: component)
228228
}
229229
}
230+
231+
// If the path resolved without opening any new fd (e.g. "."),
232+
// dup to avoid returning an aliased fd to the caller.
233+
if baseFd.rawValue == startFd.rawValue {
234+
baseFd = try startFd.open(
235+
at: ".", mode, options: options, permissions: permissions
236+
)
237+
}
238+
230239
resultFd = self.baseFd
231240
return self.baseFd
232241
}

Sources/WASI/Platform/SandboxPrimitives/OpenParent.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ struct SplitPath: ~Copyable {
8888
var basename: String
8989
var isTemporary: Bool
9090

91+
borrowing func withFields<R>(_ body: (FileDescriptor, String) throws -> R) rethrows -> R {
92+
try body(parentFd, basename)
93+
}
94+
9195
deinit {
9296
if isTemporary {
9397
try? parentFd.close()

0 commit comments

Comments
 (0)