From e96c38127fef119880a53eb9f07ac68927547121 Mon Sep 17 00:00:00 2001 From: Brandon Payton Date: Wed, 22 Jul 2026 15:00:56 -0400 Subject: [PATCH] [POSIX] Preserve directory streams when rewind fails Open the replacement iterator before retiring the active stream, and keep the old state authoritative if opening or closing the replacement sequence fails. Add direct host-bridge coverage for throwing close calls, numeric handle reuse, and partial Wasm marshalling so pending entries cannot leak or disappear across retries. --- crates/kernel/src/syscalls.rs | 79 +++++++++++++++++++++++- docs/posix-status.md | 2 +- host/src/types.ts | 4 ++ host/test/readdir-atomicity.test.ts | 93 +++++++++++++++++++++++++++++ 4 files changed, 176 insertions(+), 2 deletions(-) diff --git a/crates/kernel/src/syscalls.rs b/crates/kernel/src/syscalls.rs index a027fc1064..fdd97213c1 100644 --- a/crates/kernel/src/syscalls.rs +++ b/crates/kernel/src/syscalls.rs @@ -6344,9 +6344,18 @@ pub fn sys_rewinddir( let path = stream.path.clone(); let old_handle = stream.host_handle; - host.host_closedir(old_handle)?; + // Construct the replacement before retiring the current iterator. A + // transient reopen failure must not leave the live DirStream pointing at + // a handle that we already closed. let new_handle = host.host_opendir(&path)?; + if let Err(err) = host.host_closedir(old_handle) { + // The old iterator remains authoritative when its close fails. Do + // not leak the replacement that never became visible to the stream. + let _ = host.host_closedir(new_handle); + return Err(err); + } + let stream = proc .dir_streams .get_mut(idx) @@ -15400,6 +15409,7 @@ mod tests { dir_entry_count: usize, // total number of mock entries dir_entry_names: Option>>, dir_opendir_error: Option, + dir_closedir_error_once: Option, dir_readdir_error: Option<(usize, Errno)>, sigsuspend_signal: u32, sigsuspend_error: bool, @@ -15466,6 +15476,7 @@ mod tests { dir_entry_count: 1, dir_entry_names: None, dir_opendir_error: None, + dir_closedir_error_once: None, dir_readdir_error: None, sigsuspend_signal: 0, sigsuspend_error: false, @@ -15940,6 +15951,9 @@ mod tests { } fn host_closedir(&mut self, handle: i64) -> Result<(), Errno> { + if let Some(err) = self.dir_closedir_error_once.take() { + return Err(err); + } self.dir_entry_indices.remove(&handle); self.closed_dir_handles.push(handle); Ok(()) @@ -18917,6 +18931,69 @@ mod tests { sys_closedir(&mut proc, &mut host, dh).unwrap(); } + #[test] + fn rewinddir_reopen_failure_preserves_the_live_iterator() { + let mut proc = Process::new(1); + let mut host = MockHostIO::new(); + let dh = sys_opendir(&mut proc, &mut host, b"/tmp").unwrap(); + let original = proc.dir_streams[dh as usize].as_mut().unwrap(); + original.position = 7; + original.synth_dot_state = 2; + assert_eq!(original.host_handle, 200); + + host.dir_opendir_error = Some(Errno::EACCES); + assert_eq!( + sys_rewinddir(&mut proc, &mut host, dh), + Err(Errno::EACCES), + ); + + let preserved = proc.dir_streams[dh as usize].as_ref().unwrap(); + assert_eq!(preserved.host_handle, 200); + assert_eq!(preserved.position, 7); + assert_eq!(preserved.synth_dot_state, 2); + assert!(host.closed_dir_handles.is_empty()); + + host.dir_opendir_error = None; + sys_rewinddir(&mut proc, &mut host, dh).unwrap(); + let rewound = proc.dir_streams[dh as usize].as_ref().unwrap(); + assert_eq!(rewound.host_handle, 201); + assert_eq!(rewound.position, 0); + assert_eq!(rewound.synth_dot_state, 0); + assert_eq!(host.closed_dir_handles, [200]); + + sys_closedir(&mut proc, &mut host, dh).unwrap(); + assert_eq!(host.closed_dir_handles, [200, 201]); + } + + #[test] + fn rewinddir_close_failure_discards_the_replacement() { + let mut proc = Process::new(1); + let mut host = MockHostIO::new(); + let dh = sys_opendir(&mut proc, &mut host, b"/tmp").unwrap(); + let original = proc.dir_streams[dh as usize].as_mut().unwrap(); + original.position = 7; + original.synth_dot_state = 2; + + host.dir_closedir_error_once = Some(Errno::EIO); + assert_eq!(sys_rewinddir(&mut proc, &mut host, dh), Err(Errno::EIO)); + + let preserved = proc.dir_streams[dh as usize].as_ref().unwrap(); + assert_eq!(preserved.host_handle, 200); + assert_eq!(preserved.position, 7); + assert_eq!(preserved.synth_dot_state, 2); + assert_eq!(host.closed_dir_handles, [201]); + + sys_rewinddir(&mut proc, &mut host, dh).unwrap(); + let rewound = proc.dir_streams[dh as usize].as_ref().unwrap(); + assert_eq!(rewound.host_handle, 202); + assert_eq!(rewound.position, 0); + assert_eq!(rewound.synth_dot_state, 0); + assert_eq!(host.closed_dir_handles, [201, 200]); + + sys_closedir(&mut proc, &mut host, dh).unwrap(); + assert_eq!(host.closed_dir_handles, [201, 200, 202]); + } + #[test] fn test_telldir_returns_position() { let mut proc = Process::new(1); diff --git a/docs/posix-status.md b/docs/posix-status.md index 119710f9ab..be1ad19b5d 100644 --- a/docs/posix-status.md +++ b/docs/posix-status.md @@ -214,7 +214,7 @@ to a different directory than the original OFD. | `opendir()` | Partial | Host-delegated via DirStream table. Entry-at-a-time iteration. Stores resolved path for rewinddir. | | `readdir()` | Full | Returns WasmDirent (d_ino, d_type, d_namlen) + name buffer. Synthesizes "." and ".." entries before host entries. Tracks position for telldir/seekdir. | | `closedir()` | Full | Frees DirStream slot, delegates to host. | -| `rewinddir()` | Full | Closes and reopens directory via stored path. Resets position to 0. | +| `rewinddir()` | Full | Reopens the directory via its stored path and resets the position to zero. The replacement is opened before the live iterator is retired, so a failed reopen leaves the previous stream and position intact. | | `telldir()` | Full | Returns current position counter from DirStream. | | `seekdir()` | Full | Rewinds and skips entries to reach target position. | | `mkdir()` | Partial | Host-delegated. Relative paths resolved via kernel cwd. umask applied to mode. | diff --git a/host/src/types.ts b/host/src/types.ts index 3b04c2982b..32742a5fac 100644 --- a/host/src/types.ts +++ b/host/src/types.ts @@ -108,6 +108,10 @@ export interface PlatformIO { utimensat(path: string, atimeSec: number, atimeNsec: number, mtimeSec: number, mtimeNsec: number): void; // Directory iteration + /** + * Open a directory and return an opaque handle. A handle must not be reused + * while its previous directory iterator is still live. + */ opendir(path: string): number; /** * Return and consume the next entry. If this throws, the iterator must stay diff --git a/host/test/readdir-atomicity.test.ts b/host/test/readdir-atomicity.test.ts index 1263c1e67d..df7f9624af 100644 --- a/host/test/readdir-atomicity.test.ts +++ b/host/test/readdir-atomicity.test.ts @@ -93,4 +93,97 @@ describe("host readdir retry atomicity", () => { ), ).toBe("new-iterator"); }); + + it("clears a staged entry even when the backend close fails", () => { + const { io, kernel, memory } = createKernelBridge([ + { name: "old-iterator", type: 8, ino: 1 }, + ]); + io.closedir.mockImplementationOnce(() => { + throw new Error("injected close failure"); + }); + const bridge = kernel as unknown as { + pendingDirectoryEntries: Map< + number, + { name: string; type: number; ino: number } + >; + hostReaddir: ( + handle: bigint, + direntPtr: number, + namePtr: number, + nameLen: number, + ) => number; + hostClosedir: (handle: bigint) => number; + }; + + expect( + bridge.hostReaddir(7n, memory.buffer.byteLength - 4, 128, 64), + ).toBeLessThan(0); + expect(bridge.pendingDirectoryEntries.size).toBe(1); + expect(bridge.hostClosedir(7n)).toBeLessThan(0); + expect(bridge.pendingDirectoryEntries.size).toBe(0); + expect(io.closedir).toHaveBeenCalledWith(7); + }); + + it("drops stale transport state when opendir returns a reused handle", () => { + const { io, kernel, memory } = createKernelBridge([ + { name: "old-iterator", type: 8, ino: 1 }, + { name: "new-iterator", type: 4, ino: 2 }, + ]); + const bridge = kernel as unknown as { + hostOpendir: (pathPtr: number, pathLen: number) => bigint; + hostReaddir: ( + handle: bigint, + direntPtr: number, + namePtr: number, + nameLen: number, + ) => number; + }; + + expect( + bridge.hostReaddir(7n, memory.buffer.byteLength - 4, 128, 64), + ).toBeLessThan(0); + + new Uint8Array(memory.buffer, 256, 4).set( + new TextEncoder().encode("/tmp"), + ); + expect(bridge.hostOpendir(256, 4)).toBe(7n); + expect(bridge.hostReaddir(7n, 0, 128, 64)).toBe(1); + + expect(io.opendir).toHaveBeenCalledWith("/tmp"); + expect(io.readdir).toHaveBeenCalledTimes(2); + expect( + new TextDecoder().decode( + new Uint8Array(memory.buffer, 128, "new-iterator".length), + ), + ).toBe("new-iterator"); + }); + + it("replays an entry when the name write fails after metadata was written", () => { + const entry = { name: "retry-name", type: 8, ino: 42 }; + const { io, kernel, memory } = createKernelBridge([entry]); + const hostReaddir = ( + kernel as unknown as { + hostReaddir: ( + handle: bigint, + direntPtr: number, + namePtr: number, + nameLen: number, + ) => number; + } + ).hostReaddir.bind(kernel); + + expect( + hostReaddir(7n, 0, memory.buffer.byteLength - 2, entry.name.length), + ).toBeLessThan(0); + expect(new DataView(memory.buffer).getBigUint64(0, true)).toBe(42n); + expect(io.readdir).toHaveBeenCalledTimes(1); + + expect(hostReaddir(7n, 0, 128, entry.name.length)).toBe(1); + expect(io.readdir).toHaveBeenCalledTimes(1); + expect( + new TextDecoder().decode( + new Uint8Array(memory.buffer, 128, entry.name.length), + ), + ).toBe(entry.name); + }); });