| author |  Ryan Liptak <squeek502@hotmail.com> 2022-07-25 06:14:25-07:00 |
| committer |  GitHub <noreply@github.com> 2022-07-25 16:14:25+03:00 |
| log | 75e5b38410e81ddf21ba37a874ee2bb13dea7477 |
| tree | 86bc98c9bf81d13ca30e676e533d58c956196b72 |
| parent | 8f3ab96b0e8cd22dc98f9fe352c5d4fff491dd0e |
| signature |  Signed by PGP key 4AEE18F83AFDEB23 |
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 #122112 files changed, 26 insertions(+), 0 deletions(-)
lib/std/fs.zig+2
| ... | @@ -607,6 +607,7 @@ pub const IterableDir = struct { | ... | @@ -607,6 +607,7 @@ pub const IterableDir = struct { |
| 607 | .BADF => unreachable, // Dir is invalid or was opened without iteration ability | 607 | .BADF => unreachable, // Dir is invalid or was opened without iteration ability |
| 608 | .FAULT => unreachable, | 608 | .FAULT => unreachable, |
| 609 | .NOTDIR => unreachable, | 609 | .NOTDIR => unreachable, |
| 610 | .NOENT => return null, // The directory being iterated was deleted during iteration. | ||
| 610 | .INVAL => return error.Unexpected, // Linux may in some cases return EINVAL when reading /proc/$PID/net. | 611 | .INVAL => return error.Unexpected, // Linux may in some cases return EINVAL when reading /proc/$PID/net. |
| 611 | else => |err| return os.unexpectedErrno(err), | 612 | else => |err| return os.unexpectedErrno(err), |
| 612 | } | 613 | } |
| ... | @@ -741,6 +742,7 @@ pub const IterableDir = struct { | ... | @@ -741,6 +742,7 @@ pub const IterableDir = struct { |
| 741 | .FAULT => unreachable, | 742 | .FAULT => unreachable, |
| 742 | .NOTDIR => unreachable, | 743 | .NOTDIR => unreachable, |
| 743 | .INVAL => unreachable, | 744 | .INVAL => unreachable, |
| 745 | .NOENT => return null, // The directory being iterated was deleted during iteration. | ||
| 744 | .NOTCAPABLE => return error.AccessDenied, | 746 | .NOTCAPABLE => return error.AccessDenied, |
| 745 | else => |err| return os.unexpectedErrno(err), | 747 | else => |err| return os.unexpectedErrno(err), |
| 746 | } | 748 | } |
lib/std/fs/test.zig+24
| ... | @@ -219,6 +219,30 @@ test "Dir.Iterator twice" { | ... | @@ -219,6 +219,30 @@ test "Dir.Iterator twice" { |
| 219 | } | 219 | } |
| 220 | } | 220 | } |
| 221 | 221 | ||
| 222 | test "Dir.Iterator but dir is deleted during iteration" { | ||
| 223 | var tmp = std.testing.tmpDir(.{}); | ||
| 224 | defer tmp.cleanup(); | ||
| 225 | |||
| 226 | // Create directory and setup an iterator for it | ||
| 227 | var iterable_subdir = try tmp.dir.makeOpenPathIterable("subdir", .{}); | ||
| 228 | defer iterable_subdir.close(); | ||
| 229 | |||
| 230 | var iterator = iterable_subdir.iterate(); | ||
| 231 | |||
| 232 | // Create something to iterate over within the subdir | ||
| 233 | try tmp.dir.makePath("subdir/b"); | ||
| 234 | |||
| 235 | // Then, before iterating, delete the directory that we're iterating. | ||
| 236 | // This is a contrived reproduction, but this could happen outside of the program, in another thread, etc. | ||
| 237 | // If we get an error while trying to delete, we can skip this test (this will happen on platforms | ||
| 238 | // like Windows which will give FileBusy if the directory is currently open for iteration). | ||
| 239 | tmp.dir.deleteTree("subdir") catch return error.SkipZigTest; | ||
| 240 | |||
| 241 | // Now, when we try to iterate, the next call should return null immediately. | ||
| 242 | const entry = try iterator.next(); | ||
| 243 | try std.testing.expect(entry == null); | ||
| 244 | } | ||
| 245 | |||
| 222 | fn entryEql(lhs: IterableDir.Entry, rhs: IterableDir.Entry) bool { | 246 | fn entryEql(lhs: IterableDir.Entry, rhs: IterableDir.Entry) bool { |
| 223 | return mem.eql(u8, lhs.name, rhs.name) and lhs.kind == rhs.kind; | 247 | return mem.eql(u8, lhs.name, rhs.name) and lhs.kind == rhs.kind; |
| 224 | } | 248 | } |