diff options
| author | Andrew Kelley <andrew@ziglang.org> | 2021-12-09 21:14:39 -0700 |
|---|---|---|
| committer | Andrew Kelley <andrew@ziglang.org> | 2021-12-09 22:10:38 -0700 |
| commit | fdbb329d1009b4c7b23552e65c0344e3ce52b459 (patch) | |
| tree | ba83854a4725344ffb9132589ca577b2f947a439 | |
| parent | 4da83feccb4b0e59afdcce9796b08cc4fc8346ae (diff) | |
| download | zig-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.zig | 34 |
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. |
