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); + }); });