authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2024-12-11 14:57:11-05:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2024-12-11 14:57:11-05:00
log3670910f20b56eda41a27ed7c4bb887f870cfc5d
tree34e964ab79340005fe211292876fb8a0ec963b30
parent295c5a64f5746a10259b5fb0799415db023975c7
parent7ff42eff914e2e501f570bb8c530719bb3a2521a
signaturebadge-check Signed by PGP key B5690EEEBB952194

Merge pull request #22202 from ziglang/Cache.hit

std.Build.Cache.hit: more discipline in error handling

6 files changed, 201 insertions(+), 49 deletions(-)

lib/std/Build/Cache.zig+123-31
...@@ -40,7 +40,7 @@ pub fn addPrefix(cache: *Cache, directory: Directory) void {...@@ -40,7 +40,7 @@ pub fn addPrefix(cache: *Cache, directory: Directory) void {
4040
41/// Be sure to call `Manifest.deinit` after successful initialization.41/// Be sure to call `Manifest.deinit` after successful initialization.
42pub fn obtain(cache: *Cache) Manifest {42pub fn obtain(cache: *Cache) Manifest {
43 return Manifest{43 return .{
44 .cache = cache,44 .cache = cache,
45 .hash = cache.hash,45 .hash = cache.hash,
46 .manifest_file = null,46 .manifest_file = null,
...@@ -99,9 +99,9 @@ fn findPrefixResolved(cache: *const Cache, resolved_path: []u8) !PrefixedPath {...@@ -99,9 +99,9 @@ fn findPrefixResolved(cache: *const Cache, resolved_path: []u8) !PrefixedPath {
99}99}
100100
101fn getPrefixSubpath(allocator: Allocator, prefix: []const u8, path: []u8) ![]u8 {101fn getPrefixSubpath(allocator: Allocator, prefix: []const u8, path: []u8) ![]u8 {
102 const relative = try std.fs.path.relative(allocator, prefix, path);102 const relative = try fs.path.relative(allocator, prefix, path);
103 errdefer allocator.free(relative);103 errdefer allocator.free(relative);
104 var component_iterator = std.fs.path.NativeComponentIterator.init(relative) catch {104 var component_iterator = fs.path.NativeComponentIterator.init(relative) catch {
105 return error.NotASubPath;105 return error.NotASubPath;
106 };106 };
107 if (component_iterator.root() != null) {107 if (component_iterator.root() != null) {
...@@ -327,13 +327,27 @@ pub const Manifest = struct {...@@ -327,13 +327,27 @@ pub const Manifest = struct {
327 want_refresh_timestamp: bool = true,327 want_refresh_timestamp: bool = true,
328 files: Files = .{},328 files: Files = .{},
329 hex_digest: HexDigest,329 hex_digest: HexDigest,
330 /// Populated when hit() returns an error because of one330 diagnostic: Diagnostic = .none,
331 /// of the files listed in the manifest.
332 failed_file_index: ?usize = null,
333 /// Keeps track of the last time we performed a file system write to observe331 /// Keeps track of the last time we performed a file system write to observe
334 /// what time the file system thinks it is, according to its own granularity.332 /// what time the file system thinks it is, according to its own granularity.
335 recent_problematic_timestamp: i128 = 0,333 recent_problematic_timestamp: i128 = 0,
336334
335 pub const Diagnostic = union(enum) {
336 none,
337 manifest_create: fs.File.OpenError,
338 manifest_read: fs.File.ReadError,
339 manifest_lock: fs.File.LockError,
340 file_open: FileOp,
341 file_stat: FileOp,
342 file_read: FileOp,
343 file_hash: FileOp,
344
345 pub const FileOp = struct {
346 file_index: usize,
347 err: anyerror,
348 };
349 };
350
337 pub const Files = std.ArrayHashMapUnmanaged(File, void, FilesContext, false);351 pub const Files = std.ArrayHashMapUnmanaged(File, void, FilesContext, false);
338352
339 pub const FilesContext = struct {353 pub const FilesContext = struct {
...@@ -452,6 +466,15 @@ pub const Manifest = struct {...@@ -452,6 +466,15 @@ pub const Manifest = struct {
452 return self.addDepFileMaybePost(dir, dep_file_basename);466 return self.addDepFileMaybePost(dir, dep_file_basename);
453 }467 }
454468
469 pub const HitError = error{
470 /// Unable to check the cache for a reason that has been recorded into
471 /// the `diagnostic` field.
472 CacheCheckFailed,
473 /// A cache manifest file exists however it could not be parsed.
474 InvalidFormat,
475 OutOfMemory,
476 };
477
455 /// Check the cache to see if the input exists in it. If it exists, returns `true`.478 /// Check the cache to see if the input exists in it. If it exists, returns `true`.
456 /// A hex encoding of its hash is available by calling `final`.479 /// A hex encoding of its hash is available by calling `final`.
457 ///480 ///
...@@ -464,11 +487,11 @@ pub const Manifest = struct {...@@ -464,11 +487,11 @@ pub const Manifest = struct {
464 /// The lock on the manifest file is released when `deinit` is called. As another487 /// The lock on the manifest file is released when `deinit` is called. As another
465 /// option, one may call `toOwnedLock` to obtain a smaller object which can represent488 /// option, one may call `toOwnedLock` to obtain a smaller object which can represent
466 /// the lock. `deinit` is safe to call whether or not `toOwnedLock` has been called.489 /// the lock. `deinit` is safe to call whether or not `toOwnedLock` has been called.
467 pub fn hit(self: *Manifest) !bool {490 pub fn hit(self: *Manifest) HitError!bool {
468 const gpa = self.cache.gpa;491 const gpa = self.cache.gpa;
469 assert(self.manifest_file == null);492 assert(self.manifest_file == null);
470493
471 self.failed_file_index = null;494 self.diagnostic = .none;
472495
473 const ext = ".txt";496 const ext = ".txt";
474 var manifest_file_path: [hex_digest_len + ext.len]u8 = undefined;497 var manifest_file_path: [hex_digest_len + ext.len]u8 = undefined;
...@@ -496,17 +519,56 @@ pub const Manifest = struct {...@@ -496,17 +519,56 @@ pub const Manifest = struct {
496 break;519 break;
497 } else |err| switch (err) {520 } else |err| switch (err) {
498 error.WouldBlock => {521 error.WouldBlock => {
499 self.manifest_file = try self.cache.manifest_dir.openFile(&manifest_file_path, .{522 self.manifest_file = self.cache.manifest_dir.openFile(&manifest_file_path, .{
500 .mode = .read_write,523 .mode = .read_write,
501 .lock = .shared,524 .lock = .shared,
502 });525 }) catch |e| {
526 self.diagnostic = .{ .manifest_create = e };
527 return error.CacheCheckFailed;
528 };
503 break;529 break;
504 },530 },
505 // There are no dir components, so you would think that this was531 error.FileNotFound => {
506 // unreachable, however we have observed on macOS two processes racing532 // There are no dir components, so the only possibility
507 // to do openat() with O_CREAT manifest in ENOENT.533 // should be that the directory behind the handle has been
508 error.FileNotFound => continue,534 // deleted, however we have observed on macOS two processes
509 else => |e| return e,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 },
568 else => |e| {
569 self.diagnostic = .{ .manifest_create = e };
570 return error.CacheCheckFailed;
571 },
510 }572 }
511 }573 }
512574
...@@ -514,7 +576,14 @@ pub const Manifest = struct {...@@ -514,7 +576,14 @@ pub const Manifest = struct {
514576
515 const input_file_count = self.files.entries.len;577 const input_file_count = self.files.entries.len;
516 while (true) : (self.unhit(bin_digest, input_file_count)) {578 while (true) : (self.unhit(bin_digest, input_file_count)) {
517 const file_contents = try self.manifest_file.?.reader().readAllAlloc(gpa, manifest_file_size_max);579 const file_contents = self.manifest_file.?.reader().readAllAlloc(gpa, manifest_file_size_max) catch |err| switch (err) {
580 error.OutOfMemory => return error.OutOfMemory,
581 error.StreamTooLong => return error.OutOfMemory,
582 else => |e| {
583 self.diagnostic = .{ .manifest_read = e };
584 return error.CacheCheckFailed;
585 },
586 };
518 defer gpa.free(file_contents);587 defer gpa.free(file_contents);
519588
520 var any_file_changed = false;589 var any_file_changed = false;
...@@ -526,8 +595,11 @@ pub const Manifest = struct {...@@ -526,8 +595,11 @@ pub const Manifest = struct {
526 while (idx < input_file_count) : (idx += 1) {595 while (idx < input_file_count) : (idx += 1) {
527 const ch_file = &self.files.keys()[idx];596 const ch_file = &self.files.keys()[idx];
528 self.populateFileHash(ch_file) catch |err| {597 self.populateFileHash(ch_file) catch |err| {
529 self.failed_file_index = idx;598 self.diagnostic = .{ .file_hash = .{
530 return err;599 .file_index = idx,
600 .err = err,
601 } };
602 return error.CacheCheckFailed;
531 };603 };
532 }604 }
533 return false;605 return false;
...@@ -605,13 +677,22 @@ pub const Manifest = struct {...@@ -605,13 +677,22 @@ pub const Manifest = struct {
605 if (try self.upgradeToExclusiveLock()) continue;677 if (try self.upgradeToExclusiveLock()) continue;
606 return false;678 return false;
607 },679 },
608 else => return error.CacheUnavailable,680 else => |e| {
681 self.diagnostic = .{ .file_open = .{
682 .file_index = idx,
683 .err = e,
684 } };
685 return error.CacheCheckFailed;
686 },
609 };687 };
610 defer this_file.close();688 defer this_file.close();
611689
612 const actual_stat = this_file.stat() catch |err| {690 const actual_stat = this_file.stat() catch |err| {
613 self.failed_file_index = idx;691 self.diagnostic = .{ .file_stat = .{
614 return err;692 .file_index = idx,
693 .err = err,
694 } };
695 return error.CacheCheckFailed;
615 };696 };
616 const size_match = actual_stat.size == cache_hash_file.stat.size;697 const size_match = actual_stat.size == cache_hash_file.stat.size;
617 const mtime_match = actual_stat.mtime == cache_hash_file.stat.mtime;698 const mtime_match = actual_stat.mtime == cache_hash_file.stat.mtime;
...@@ -634,8 +715,11 @@ pub const Manifest = struct {...@@ -634,8 +715,11 @@ pub const Manifest = struct {
634715
635 var actual_digest: BinDigest = undefined;716 var actual_digest: BinDigest = undefined;
636 hashFile(this_file, &actual_digest) catch |err| {717 hashFile(this_file, &actual_digest) catch |err| {
637 self.failed_file_index = idx;718 self.diagnostic = .{ .file_read = .{
638 return err;719 .file_index = idx,
720 .err = err,
721 } };
722 return error.CacheCheckFailed;
639 };723 };
640724
641 if (!mem.eql(u8, &cache_hash_file.bin_digest, &actual_digest)) {725 if (!mem.eql(u8, &cache_hash_file.bin_digest, &actual_digest)) {
...@@ -662,17 +746,22 @@ pub const Manifest = struct {...@@ -662,17 +746,22 @@ pub const Manifest = struct {
662 if (try self.upgradeToExclusiveLock()) continue;746 if (try self.upgradeToExclusiveLock()) continue;
663 self.manifest_dirty = true;747 self.manifest_dirty = true;
664 while (idx < input_file_count) : (idx += 1) {748 while (idx < input_file_count) : (idx += 1) {
665 const ch_file = &self.files.keys()[idx];749 self.populateFileHash(&self.files.keys()[idx]) catch |err| {
666 self.populateFileHash(ch_file) catch |err| {750 self.diagnostic = .{ .file_hash = .{
667 self.failed_file_index = idx;751 .file_index = idx,
668 return err;752 .err = err,
753 } };
754 return error.CacheCheckFailed;
669 };755 };
670 }756 }
671 return false;757 return false;
672 }758 }
673759
674 if (self.want_shared_lock) {760 if (self.want_shared_lock) {
675 try self.downgradeToSharedLock();761 self.downgradeToSharedLock() catch |err| {
762 self.diagnostic = .{ .manifest_lock = err };
763 return error.CacheCheckFailed;
764 };
676 }765 }
677766
678 return true;767 return true;
...@@ -1010,7 +1099,7 @@ pub const Manifest = struct {...@@ -1010,7 +1099,7 @@ pub const Manifest = struct {
1010 self.have_exclusive_lock = false;1099 self.have_exclusive_lock = false;
1011 }1100 }
10121101
1013 fn upgradeToExclusiveLock(self: *Manifest) !bool {1102 fn upgradeToExclusiveLock(self: *Manifest) error{CacheCheckFailed}!bool {
1014 if (self.have_exclusive_lock) return false;1103 if (self.have_exclusive_lock) return false;
1015 assert(self.manifest_file != null);1104 assert(self.manifest_file != null);
10161105
...@@ -1022,7 +1111,10 @@ pub const Manifest = struct {...@@ -1022,7 +1111,10 @@ pub const Manifest = struct {
1022 // Here we intentionally have a period where the lock is released, in case there are1111 // Here we intentionally have a period where the lock is released, in case there are
1023 // other processes holding a shared lock.1112 // other processes holding a shared lock.
1024 manifest_file.unlock();1113 manifest_file.unlock();
1025 try manifest_file.lock(.exclusive);1114 manifest_file.lock(.exclusive) catch |err| {
1115 self.diagnostic = .{ .manifest_lock = err };
1116 return error.CacheCheckFailed;
1117 };
1026 }1118 }
1027 self.have_exclusive_lock = true;1119 self.have_exclusive_lock = true;
1028 return true;1120 return true;
...@@ -1132,7 +1224,7 @@ pub fn writeSmallFile(dir: fs.Dir, sub_path: []const u8, data: []const u8) !void...@@ -1132,7 +1224,7 @@ pub fn writeSmallFile(dir: fs.Dir, sub_path: []const u8, data: []const u8) !void
1132 }1224 }
1133}1225}
11341226
1135fn hashFile(file: fs.File, bin_digest: *[Hasher.mac_length]u8) !void {1227fn hashFile(file: fs.File, bin_digest: *[Hasher.mac_length]u8) fs.File.PReadError!void {
1136 var buf: [1024]u8 = undefined;1228 var buf: [1024]u8 = undefined;
1137 var hasher = hasher_init;1229 var hasher = hasher_init;
1138 var off: u64 = 0;1230 var off: u64 = 0;
lib/std/Build/Step.zig+18-5
...@@ -754,11 +754,24 @@ pub fn cacheHitAndWatch(s: *Step, man: *Build.Cache.Manifest) !bool {...@@ -754,11 +754,24 @@ pub fn cacheHitAndWatch(s: *Step, man: *Build.Cache.Manifest) !bool {
754 return is_hit;754 return is_hit;
755}755}
756756
757fn failWithCacheError(s: *Step, man: *const Build.Cache.Manifest, err: anyerror) anyerror {757fn failWithCacheError(s: *Step, man: *const Build.Cache.Manifest, err: Build.Cache.Manifest.HitError) error{ OutOfMemory, MakeFailed } {
758 const i = man.failed_file_index orelse return err;758 switch (err) {
759 const pp = man.files.keys()[i].prefixed_path;759 error.CacheCheckFailed => switch (man.diagnostic) {
760 const prefix = man.cache.prefixes()[pp.prefix].path orelse "";760 .none => unreachable,
761 return s.fail("{s}: {s}/{s}", .{ @errorName(err), prefix, pp.sub_path });761 .manifest_create, .manifest_read, .manifest_lock => |e| return s.fail("failed to check cache: {s} {s}", .{
762 @tagName(man.diagnostic), @errorName(e),
763 }),
764 .file_open, .file_stat, .file_read, .file_hash => |op| {
765 const pp = man.files.keys()[op.file_index].prefixed_path;
766 const prefix = man.cache.prefixes()[pp.prefix].path orelse "";
767 return s.fail("failed to check cache: '{s}{s}' {s} {s}", .{
768 prefix, pp.sub_path, @tagName(man.diagnostic), @errorName(op.err),
769 });
770 },
771 },
772 error.OutOfMemory => return error.OutOfMemory,
773 error.InvalidFormat => return s.fail("failed to check cache: invalid manifest file format", .{}),
774 }
762}775}
763776
764/// Prefer `writeManifestAndWatch` unless you already added watch inputs777/// Prefer `writeManifestAndWatch` unless you already added watch inputs
lib/std/posix.zig+6
...@@ -1537,6 +1537,12 @@ pub const OpenError = error{...@@ -1537,6 +1537,12 @@ pub const OpenError = error{
1537 ProcessFdQuotaExceeded,1537 ProcessFdQuotaExceeded,
1538 SystemFdQuotaExceeded,1538 SystemFdQuotaExceeded,
1539 NoDevice,1539 NoDevice,
1540 /// Either:
1541 /// * One of the path components does not exist.
1542 /// * Cwd was used, but cwd 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`.
1540 FileNotFound,1546 FileNotFound,
15411547
1542 /// The path exceeded `max_path_bytes` bytes.1548 /// The path exceeded `max_path_bytes` bytes.
src/Compilation.zig+23-8
...@@ -2048,15 +2048,30 @@ pub fn update(comp: *Compilation, main_progress_node: std.Progress.Node) !void {...@@ -2048,15 +2048,30 @@ pub fn update(comp: *Compilation, main_progress_node: std.Progress.Node) !void {
2048 whole.cache_manifest = &man;2048 whole.cache_manifest = &man;
2049 try addNonIncrementalStuffToCacheManifest(comp, arena, &man);2049 try addNonIncrementalStuffToCacheManifest(comp, arena, &man);
20502050
2051 const is_hit = man.hit() catch |err| {2051 const is_hit = man.hit() catch |err| switch (err) {
2052 const i = man.failed_file_index orelse return err;2052 error.CacheCheckFailed => switch (man.diagnostic) {
2053 const pp = man.files.keys()[i].prefixed_path;2053 .none => unreachable,
2054 const prefix = man.cache.prefixes()[pp.prefix];2054 .manifest_create, .manifest_read, .manifest_lock => |e| return comp.setMiscFailure(
2055 return comp.setMiscFailure(2055 .check_whole_cache,
2056 "failed to check cache: {s} {s}",
2057 .{ @tagName(man.diagnostic), @errorName(e) },
2058 ),
2059 .file_open, .file_stat, .file_read, .file_hash => |op| {
2060 const pp = man.files.keys()[op.file_index].prefixed_path;
2061 const prefix = man.cache.prefixes()[pp.prefix];
2062 return comp.setMiscFailure(
2063 .check_whole_cache,
2064 "failed to check cache: '{}{s}' {s} {s}",
2065 .{ prefix, pp.sub_path, @tagName(man.diagnostic), @errorName(op.err) },
2066 );
2067 },
2068 },
2069 error.OutOfMemory => return error.OutOfMemory,
2070 error.InvalidFormat => return comp.setMiscFailure(
2056 .check_whole_cache,2071 .check_whole_cache,
2057 "unable to check cache: stat file '{}{s}' failed: {s}",2072 "failed to check cache: invalid manifest file format",
2058 .{ prefix, pp.sub_path, @errorName(err) },2073 .{},
2059 );2074 ),
2060 };2075 };
2061 if (is_hit) {2076 if (is_hit) {
2062 // In this case the cache hit contains the full set of file system inputs. Nice!2077 // In this case the cache hit contains the full set of file system inputs. Nice!
src/Zcu/PerThread.zig+30-4
...@@ -135,10 +135,36 @@ pub fn astGenFile(...@@ -135,10 +135,36 @@ pub fn astGenFile(
135 error.PipeBusy => unreachable, // it's not a pipe135 error.PipeBusy => unreachable, // it's not a pipe
136 error.NoDevice => unreachable, // it's not a pipe136 error.NoDevice => unreachable, // it's not a pipe
137 error.WouldBlock => unreachable, // not asking for non-blocking I/O137 error.WouldBlock => unreachable, // not asking for non-blocking I/O
138 // There are no dir components, so you would think that this was138 error.FileNotFound => {
139 // unreachable, however we have observed on macOS two processes racing139 // There are no dir components, so the only possibility should
140 // to do openat() with O_CREAT manifest in ENOENT.140 // be that the directory behind the handle has been deleted,
141 error.FileNotFound => continue,141 // however we have observed on macOS two processes racing to do
142 // openat() with O_CREAT manifest in ENOENT.
143 //
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 };
167 },
142168
143 else => |e| return e, // Retryable errors are handled at callsite.169 else => |e| return e, // Retryable errors are handled at callsite.
144 };170 };
src/link.zig+1-1
...@@ -768,7 +768,7 @@ pub const File = struct {...@@ -768,7 +768,7 @@ pub const File = struct {
768 /// TODO audit this error set. most of these should be collapsed into one error,768 /// TODO audit this error set. most of these should be collapsed into one error,
769 /// and Diags.Flags should be updated to convey the meaning to the user.769 /// and Diags.Flags should be updated to convey the meaning to the user.
770 pub const FlushError = error{770 pub const FlushError = error{
771 CacheUnavailable,771 CacheCheckFailed,
772 CurrentWorkingDirectoryUnlinked,772 CurrentWorkingDirectoryUnlinked,
773 DivisionByZero,773 DivisionByZero,
774 DllImportLibraryNotFound,774 DllImportLibraryNotFound,