authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2020-08-18 12:44:00-07:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2020-08-18 12:44:00-07:00
loge2c741f1e7dbddabdbfc18a2520f5efa376899bc
treea64c3cf24abd931f8b87f1998de649334c65f414
parentbdb8c494188f699527e927f3a4d3b37d3823549f

std.cache_hash: additionally use file size to detect modifications

I have observed on Linux writing and reading the same file many times without the mtime changing, despite the file system having nanosecond granularity (and about 1 millisecond worth of nanoseconds passing between modifications). I am calling this a Linux Kernel Bug and adding file size to the cache hash manifest as a mitigation. As evidence, macOS does not exhibit this behavior. This means it is possible, on Linux, for a file to be added to the cache hash, and, if it is updated with the same file size, same inode, within about 1 millisecond, the cache system will give us a false positive, saying it is unmodified. I don't see any way to improve this situation without fixing the bug in the Linux kernel. closes #6082

1 files changed, 6 insertions(+), 10 deletions(-)

lib/std/cache_hash.zig+6-10
...@@ -188,11 +188,13 @@ pub const CacheHash = struct {...@@ -188,11 +188,13 @@ pub const CacheHash = struct {
188 };188 };
189189
190 var iter = mem.tokenize(line, " ");190 var iter = mem.tokenize(line, " ");
191 const size = iter.next() orelse return error.InvalidFormat;
191 const inode = iter.next() orelse return error.InvalidFormat;192 const inode = iter.next() orelse return error.InvalidFormat;
192 const mtime_nsec_str = iter.next() orelse return error.InvalidFormat;193 const mtime_nsec_str = iter.next() orelse return error.InvalidFormat;
193 const digest_str = iter.next() orelse return error.InvalidFormat;194 const digest_str = iter.next() orelse return error.InvalidFormat;
194 const file_path = iter.rest();195 const file_path = iter.rest();
195196
197 cache_hash_file.stat.size = fmt.parseInt(u64, size, 10) catch return error.InvalidFormat;
196 cache_hash_file.stat.inode = fmt.parseInt(fs.File.INode, inode, 10) catch return error.InvalidFormat;198 cache_hash_file.stat.inode = fmt.parseInt(fs.File.INode, inode, 10) catch return error.InvalidFormat;
197 cache_hash_file.stat.mtime = fmt.parseInt(i64, mtime_nsec_str, 10) catch return error.InvalidFormat;199 cache_hash_file.stat.mtime = fmt.parseInt(i64, mtime_nsec_str, 10) catch return error.InvalidFormat;
198 base64_decoder.decode(&cache_hash_file.bin_digest, digest_str) catch return error.InvalidFormat;200 base64_decoder.decode(&cache_hash_file.bin_digest, digest_str) catch return error.InvalidFormat;
...@@ -216,10 +218,11 @@ pub const CacheHash = struct {...@@ -216,10 +218,11 @@ pub const CacheHash = struct {
216 defer this_file.close();218 defer this_file.close();
217219
218 const actual_stat = try this_file.stat();220 const actual_stat = try this_file.stat();
221 const size_match = actual_stat.size == cache_hash_file.stat.size;
219 const mtime_match = actual_stat.mtime == cache_hash_file.stat.mtime;222 const mtime_match = actual_stat.mtime == cache_hash_file.stat.mtime;
220 const inode_match = actual_stat.inode == cache_hash_file.stat.inode;223 const inode_match = actual_stat.inode == cache_hash_file.stat.inode;
221224
222 if (!mtime_match or !inode_match) {225 if (!size_match or !mtime_match or !inode_match) {
223 self.manifest_dirty = true;226 self.manifest_dirty = true;
224227
225 cache_hash_file.stat = actual_stat;228 cache_hash_file.stat = actual_stat;
...@@ -392,7 +395,7 @@ pub const CacheHash = struct {...@@ -392,7 +395,7 @@ pub const CacheHash = struct {
392395
393 for (self.files.items) |file| {396 for (self.files.items) |file| {
394 base64_encoder.encode(encoded_digest[0..], &file.bin_digest);397 base64_encoder.encode(encoded_digest[0..], &file.bin_digest);
395 try outStream.print("{} {} {} {}\n", .{ file.stat.inode, file.stat.mtime, encoded_digest[0..], file.path });398 try outStream.print("{} {} {} {} {}\n", .{ file.stat.size, file.stat.inode, file.stat.mtime, encoded_digest[0..], file.path });
396 }399 }
397400
398 try self.manifest_file.?.pwriteAll(contents.items, 0);401 try self.manifest_file.?.pwriteAll(contents.items, 0);
...@@ -451,17 +454,10 @@ fn isProblematicTimestamp(fs_clock: i128) bool {...@@ -451,17 +454,10 @@ fn isProblematicTimestamp(fs_clock: i128) bool {
451 // to detect precision of seconds, because looking at the zero bits in base454 // to detect precision of seconds, because looking at the zero bits in base
452 // 2 would not detect precision of the seconds value.455 // 2 would not detect precision of the seconds value.
453 const fs_sec = @intCast(i64, @divFloor(fs_clock, std.time.ns_per_s));456 const fs_sec = @intCast(i64, @divFloor(fs_clock, std.time.ns_per_s));
454 var fs_nsec = @intCast(i64, @mod(fs_clock, std.time.ns_per_s));457 const fs_nsec = @intCast(i64, @mod(fs_clock, std.time.ns_per_s));
455 var wall_sec = @intCast(i64, @divFloor(wall_clock, std.time.ns_per_s));458 var wall_sec = @intCast(i64, @divFloor(wall_clock, std.time.ns_per_s));
456 var wall_nsec = @intCast(i64, @mod(wall_clock, std.time.ns_per_s));459 var wall_nsec = @intCast(i64, @mod(wall_clock, std.time.ns_per_s));
457460
458 if (std.Target.current.os.tag == .linux) {
459 // TODO As a temporary measure while we figure out how to solve
460 // https://github.com/ziglang/zig/issues/6082, we cut the granularity of nanoseconds
461 // by a large amount.
462 fs_nsec &= @as(i64, -1) << 23;
463 }
464
465 // First make all the least significant zero bits in the fs_clock, also zero bits in the wall clock.461 // First make all the least significant zero bits in the fs_clock, also zero bits in the wall clock.
466 if (fs_nsec == 0) {462 if (fs_nsec == 0) {
467 wall_nsec = 0;463 wall_nsec = 0;