| author |  Andrew Kelley <andrew@ziglang.org> 2024-12-10 20:44:00-08:00 |
| committer | Andrew Kelley <andrew@ziglang.org> 2024-12-11 11:56:44-08:00 |
| log | 7ff42eff914e2e501f570bb8c530719bb3a2521a |
| tree | ec5da1b9e68065a5dd1125751bb04f7292b23989 |
| parent | d37ee79535188263bd6a907eb26a48364c4c12f2 |
std.Build.Cache.hit: work around macOS kernel bug
The previous commit cast doubt upon the initial report about macOS
kernel behavior, identifying another reason that ENOENT could be
returned from file creation.
However, it is demonstrable that ENOENT can be returned for both cases:
1. create file race
2. handle refers to deleted directory
This commit re-introduces the workaround for the file creation race on
macOS however it does not unconditionally retry - it first tries again
with O_EXCL to disambiguate the error condition that has occurred.5 files changed, 69 insertions(+), 8 deletions(-)
lib/std/Build/Cache.zig+37
| ... | @@ -528,6 +528,43 @@ pub const Manifest = struct { | ... | @@ -528,6 +528,43 @@ pub const Manifest = struct { |
| 528 | }; | 528 | }; |
| 529 | break; | 529 | break; |
| 530 | }, | 530 | }, |
| 531 | error.FileNotFound => { | ||
| 532 | // There are no dir components, so the only possibility | ||
| 533 | // should be that the directory behind the handle has been | ||
| 534 | // deleted, however we have observed on macOS two processes | ||
| 535 | // racing to do openat() with O_CREAT manifest in ENOENT. | ||
| 536 | // | ||
| 537 | // As a workaround, we retry with exclusive=true which | ||
| 538 | // disambiguates by returning EEXIST, indicating original | ||
| 539 | // failure was a race, or ENOENT, indicating deletion of | ||
| 540 | // the directory of our open handle. | ||
| 541 | if (builtin.os.tag != .macos) { | ||
| 542 | self.diagnostic = .{ .manifest_create = error.FileNotFound }; | ||
| 543 | return error.CacheCheckFailed; | ||
| 544 | } | ||
| 545 | |||
| 546 | if (self.cache.manifest_dir.createFile(&manifest_file_path, .{ | ||
| 547 | .read = true, | ||
| 548 | .truncate = false, | ||
| 549 | .lock = .exclusive, | ||
| 550 | .lock_nonblocking = self.want_shared_lock, | ||
| 551 | .exclusive = true, | ||
| 552 | })) |manifest_file| { | ||
| 553 | self.manifest_file = manifest_file; | ||
| 554 | self.have_exclusive_lock = true; | ||
| 555 | break; | ||
| 556 | } else |excl_err| switch (excl_err) { | ||
| 557 | error.WouldBlock, error.PathAlreadyExists => continue, | ||
| 558 | error.FileNotFound => { | ||
| 559 | self.diagnostic = .{ .manifest_create = error.FileNotFound }; | ||
| 560 | return error.CacheCheckFailed; | ||
| 561 | }, | ||
| 562 | else => |e| { | ||
| 563 | self.diagnostic = .{ .manifest_create = e }; | ||
| 564 | return error.CacheCheckFailed; | ||
| 565 | }, | ||
| 566 | } | ||
| 567 | }, | ||
| 531 | else => |e| { | 568 | else => |e| { |
| 532 | self.diagnostic = .{ .manifest_create = e }; | 569 | self.diagnostic = .{ .manifest_create = e }; |
| 533 | return error.CacheCheckFailed; | 570 | return error.CacheCheckFailed; |
lib/std/Build/Step.zig+1-1
| ... | @@ -770,7 +770,7 @@ fn failWithCacheError(s: *Step, man: *const Build.Cache.Manifest, err: Build.Cac | ... | @@ -770,7 +770,7 @@ fn failWithCacheError(s: *Step, man: *const Build.Cache.Manifest, err: Build.Cac |
| 770 | }, | 770 | }, |
| 771 | }, | 771 | }, |
| 772 | error.OutOfMemory => return error.OutOfMemory, | 772 | error.OutOfMemory => return error.OutOfMemory, |
| 773 | error.InvalidFormat => return s.fail("failed check cache: invalid manifest file format", .{}), | 773 | error.InvalidFormat => return s.fail("failed to check cache: invalid manifest file format", .{}), |
| 774 | } | 774 | } |
| 775 | } | 775 | } |
| 776 | 776 |
lib/std/posix.zig+2
| ... | @@ -1541,6 +1541,8 @@ pub const OpenError = error{ | ... | @@ -1541,6 +1541,8 @@ pub const OpenError = error{ |
| 1541 | /// * One of the path components does not exist. | 1541 | /// * One of the path components does not exist. |
| 1542 | /// * Cwd was used, but cwd has been deleted. | 1542 | /// * Cwd was used, but cwd has been deleted. |
| 1543 | /// * The path associated with the open directory handle has been deleted. | 1543 | /// * The path associated with the open directory handle has been deleted. |
| 1544 | /// * On macOS, multiple processes or threads raced to create the same file | ||
| 1545 | /// with `O.EXCL` set to `false`. | ||
| 1544 | FileNotFound, | 1546 | FileNotFound, |
| 1545 | 1547 | ||
| 1546 | /// The path exceeded `max_path_bytes` bytes. | 1548 | /// The path exceeded `max_path_bytes` bytes. |
src/Compilation.zig+1-1
| ... | @@ -2069,7 +2069,7 @@ pub fn update(comp: *Compilation, main_progress_node: std.Progress.Node) !void { | ... | @@ -2069,7 +2069,7 @@ pub fn update(comp: *Compilation, main_progress_node: std.Progress.Node) !void { |
| 2069 | error.OutOfMemory => return error.OutOfMemory, | 2069 | error.OutOfMemory => return error.OutOfMemory, |
| 2070 | error.InvalidFormat => return comp.setMiscFailure( | 2070 | error.InvalidFormat => return comp.setMiscFailure( |
| 2071 | .check_whole_cache, | 2071 | .check_whole_cache, |
| 2072 | "failed check cache: invalid manifest file format", | 2072 | "failed to check cache: invalid manifest file format", |
| 2073 | .{}, | 2073 | .{}, |
| 2074 | ), | 2074 | ), |
| 2075 | }; | 2075 | }; |
src/Zcu/PerThread.zig+28-6
| ... | @@ -136,12 +136,34 @@ pub fn astGenFile( | ... | @@ -136,12 +136,34 @@ pub fn astGenFile( |
| 136 | error.NoDevice => unreachable, // it's not a pipe | 136 | error.NoDevice => unreachable, // it's not a pipe |
| 137 | error.WouldBlock => unreachable, // not asking for non-blocking I/O | 137 | error.WouldBlock => unreachable, // not asking for non-blocking I/O |
| 138 | error.FileNotFound => { | 138 | error.FileNotFound => { |
| 139 | // Since there are no dir components this could only occur if | 139 | // There are no dir components, so the only possibility should |
| 140 | // `zir_dir` is deleted after the compiler process obtains an | 140 | // be that the directory behind the handle has been deleted, |
| 141 | // open directory handle. | 141 | // however we have observed on macOS two processes racing to do |
| 142 | std.process.fatal("cache directory '{}' unexpectedly removed during compiler execution", .{ | 142 | // openat() with O_CREAT manifest in ENOENT. |
| 143 | cache_directory, | 143 | // |
| 144 | }); | 144 | // As a workaround, we retry with exclusive=true which |
| 145 | // disambiguates by returning EEXIST, indicating original | ||
| 146 | // failure was a race, or ENOENT, indicating deletion of the | ||
| 147 | // directory of our open handle. | ||
| 148 | if (builtin.os.tag != .macos) { | ||
| 149 | std.process.fatal("cache directory '{}' unexpectedly removed during compiler execution", .{ | ||
| 150 | cache_directory, | ||
| 151 | }); | ||
| 152 | } | ||
| 153 | break zir_dir.createFile(&hex_digest, .{ | ||
| 154 | .read = true, | ||
| 155 | .truncate = false, | ||
| 156 | .lock = lock, | ||
| 157 | .exclusive = true, | ||
| 158 | }) catch |excl_err| switch (excl_err) { | ||
| 159 | error.PathAlreadyExists => continue, | ||
| 160 | error.FileNotFound => { | ||
| 161 | std.process.fatal("cache directory '{}' unexpectedly removed during compiler execution", .{ | ||
| 162 | cache_directory, | ||
| 163 | }); | ||
| 164 | }, | ||
| 165 | else => |e| return e, | ||
| 166 | }; | ||
| 145 | }, | 167 | }, |
| 146 | 168 | ||
| 147 | else => |e| return e, // Retryable errors are handled at callsite. | 169 | else => |e| return e, // Retryable errors are handled at callsite. |