From 75e5b38410e81ddf21ba37a874ee2bb13dea7477 Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Mon, 25 Jul 2022 06:14:25 -0700 Subject: [PATCH] std.fs: End iteration on Linux/WASI during Iterator.next when hitting `ENOENT` `getdents` on Linux can return `ENOENT` if the directory referred to by the fd is deleted during iteration. Returning null when this happens makes sense because: - `ENOENT` is specific to the Linux implementation of `getdents` - On other platforms like FreeBSD, `getdents` returns `0` in this scenario, which is functionally equivalent to the `.NOENT => return null` handling on Linux - In all the usage sites of `Iterator.next` throughout the standard library, translating `ENOENT` returned from `next` as null was the best way to handle it, so the use-case for handling the exact `ENOENT` scenario specifically may not exist to a relevant extent Previously, ENOENT being returned would trigger `os.unexpectedErrno`. Closes #12211 --- lib/std/fs.zig | 2 ++ lib/std/fs/test.zig | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 1cf1d4533f..c96a118399 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -607,6 +607,7 @@ pub const IterableDir = struct { .BADF => unreachable, // Dir is invalid or was opened without iteration ability .FAULT => unreachable, .NOTDIR => unreachable, + .NOENT => return null, // The directory being iterated was deleted during iteration. .INVAL => return error.Unexpected, // Linux may in some cases return EINVAL when reading /proc/$PID/net. else => |err| return os.unexpectedErrno(err), } @@ -741,6 +742,7 @@ pub const IterableDir = struct { .FAULT => unreachable, .NOTDIR => unreachable, .INVAL => unreachable, + .NOENT => return null, // The directory being iterated was deleted during iteration. .NOTCAPABLE => return error.AccessDenied, else => |err| return os.unexpectedErrno(err), } diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index 4e2e791c9d..bedec7d4ad 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -219,6 +219,30 @@ test "Dir.Iterator twice" { } } +test "Dir.Iterator but dir is deleted during iteration" { + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + + // Create directory and setup an iterator for it + var iterable_subdir = try tmp.dir.makeOpenPathIterable("subdir", .{}); + defer iterable_subdir.close(); + + var iterator = iterable_subdir.iterate(); + + // Create something to iterate over within the subdir + try tmp.dir.makePath("subdir/b"); + + // Then, before iterating, delete the directory that we're iterating. + // This is a contrived reproduction, but this could happen outside of the program, in another thread, etc. + // If we get an error while trying to delete, we can skip this test (this will happen on platforms + // like Windows which will give FileBusy if the directory is currently open for iteration). + tmp.dir.deleteTree("subdir") catch return error.SkipZigTest; + + // Now, when we try to iterate, the next call should return null immediately. + const entry = try iterator.next(); + try std.testing.expect(entry == null); +} + fn entryEql(lhs: IterableDir.Entry, rhs: IterableDir.Entry) bool { return mem.eql(u8, lhs.name, rhs.name) and lhs.kind == rhs.kind; }