aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndrew Kelley <andrew@ziglang.org>2021-12-09 21:14:39 -0700
committerAndrew Kelley <andrew@ziglang.org>2021-12-09 22:10:38 -0700
commitfdbb329d1009b4c7b23552e65c0344e3ce52b459 (patch)
treeba83854a4725344ffb9132589ca577b2f947a439
parent4da83feccb4b0e59afdcce9796b08cc4fc8346ae (diff)
downloadzig-fdbb329d1009b4c7b23552e65c0344e3ce52b459.tar.gz
zig-fdbb329d1009b4c7b23552e65c0344e3ce52b459.zip
Cache: fix data race with is_problematic_timestamp
Previously `recent_problematic_timestamp` was unprotected and accessed potentially with multiple worker threads simultaneously. This commit protects it with atomics and also introduces a flag to prevent multiple timestamp checks from within the same call to hit(). Unfortunately the compiler-rt function __sync_val_compare_and_swap_16 is not yet implemented, so I will have to take a different strategy in a follow-up commit.
-rw-r--r--src/Cache.zig34
1 files changed, 26 insertions, 8 deletions
diff --git a/src/Cache.zig b/src/Cache.zig
index 857ddc78e3..03b44fc535 100644
--- a/src/Cache.zig
+++ b/src/Cache.zig
@@ -2,6 +2,7 @@ gpa: Allocator,
manifest_dir: fs.Dir,
hash: HashHelper = .{},
recent_problematic_timestamp: i128 = 0,
+want_refresh_timestamp: bool = true,
const Cache = @This();
const std = @import("std");
@@ -346,6 +347,12 @@ pub const Manifest = struct {
}
}
+ // Indicate that we want isProblematicTimestamp to perform a filesystem write in
+ // order to obtain a problematic timestamp for the next call. Calls after that
+ // in this same hit() function call will then use the same timestamp, to avoid
+ // writing multiple times to the filesystem.
+ @atomicStore(bool, &self.cache.want_refresh_timestamp, true, .Monotonic);
+
const file_contents = try self.manifest_file.?.reader().readAllAlloc(self.cache.gpa, manifest_file_size_max);
defer self.cache.gpa.free(file_contents);
@@ -749,18 +756,29 @@ fn hashFile(file: fs.File, bin_digest: *[Hasher.mac_length]u8) !void {
fn isProblematicTimestamp(cache: *Cache, file_time: i128) bool {
// If the file_time is prior to the most recent problematic timestamp
// then we don't need to access the filesystem.
- if (file_time < cache.recent_problematic_timestamp)
+ var ts = @atomicLoad(i128, &cache.recent_problematic_timestamp, .Monotonic);
+ if (file_time < ts)
return false;
- var file = cache.manifest_dir.createFile("timestamp", .{
- .read = true,
- .truncate = true,
- }) catch return true;
- defer file.close();
+ if (@atomicRmw(bool, &cache.want_refresh_timestamp, .Xchg, false, .Monotonic)) {
+ var file = cache.manifest_dir.createFile("timestamp", .{
+ .read = true,
+ .truncate = true,
+ }) catch return true;
+ defer file.close();
- cache.recent_problematic_timestamp = (file.stat() catch return true).mtime;
+ const new_ts = (file.stat() catch return true).mtime;
+ ts = if (@cmpxchgWeak(
+ i128,
+ &cache.recent_problematic_timestamp,
+ ts,
+ new_ts,
+ .Monotonic,
+ .Monotonic,
+ )) |race_ts| race_ts else new_ts;
+ }
- return file_time >= cache.recent_problematic_timestamp;
+ return file_time >= ts;
}
// Create/Write a file, close it, then grab its stat.mtime timestamp.