diff options
| author | Casey Banner <kcbanner@gmail.com> | 2022-12-06 23:15:54 -0500 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2022-12-06 23:15:54 -0500 |
| commit | 8ccb9a6ad327a4d7fbc321b33d4aa66a27a1f5ee (patch) | |
| tree | 13556a64c4a9af587f12605085b4c880334772e3 /src | |
| parent | 14416b522e697997f21dd634bfdc70fbff59471b (diff) | |
| download | zig-8ccb9a6ad327a4d7fbc321b33d4aa66a27a1f5ee.tar.gz zig-8ccb9a6ad327a4d7fbc321b33d4aa66a27a1f5ee.zip | |
cache: Fix LockViolation during C compilation paths (#13591)
- C compilation flows didn't hold an exclusive lock on the cache manifest file when writing to it in all cases
- On windows, explicitly unlock the file lock before closing it
Diffstat (limited to 'src')
| -rw-r--r-- | src/Cache.zig | 19 | ||||
| -rw-r--r-- | src/Compilation.zig | 4 | ||||
| -rw-r--r-- | src/main.zig | 1 |
3 files changed, 23 insertions, 1 deletions
diff --git a/src/Cache.zig b/src/Cache.zig index a645e06594..586979c187 100644 --- a/src/Cache.zig +++ b/src/Cache.zig @@ -232,6 +232,12 @@ pub const Lock = struct { manifest_file: fs.File, pub fn release(lock: *Lock) void { + if (builtin.os.tag == .windows) { + // Windows does not guarantee that locks are immediately unlocked when + // the file handle is closed. See LockFileEx documentation. + lock.manifest_file.unlock(); + } + lock.manifest_file.close(); lock.* = undefined; } @@ -554,7 +560,10 @@ pub const Manifest = struct { return false; } - try self.downgradeToSharedLock(); + if (self.want_shared_lock) { + try self.downgradeToSharedLock(); + } + return true; } @@ -866,11 +875,13 @@ pub const Manifest = struct { const manifest_file = self.manifest_file.?; try manifest_file.downgradeLock(); } + self.have_exclusive_lock = false; } fn upgradeToExclusiveLock(self: *Manifest) !void { if (self.have_exclusive_lock) return; + assert(self.manifest_file != null); // WASI does not currently support flock, so we bypass it here. // TODO: If/when flock is supported on WASI, this check should be removed. @@ -892,6 +903,7 @@ pub const Manifest = struct { const lock: Lock = .{ .manifest_file = self.manifest_file.?, }; + self.manifest_file = null; return lock; } @@ -901,6 +913,11 @@ pub const Manifest = struct { /// Don't forget to call `writeManifest` before this! pub fn deinit(self: *Manifest) void { if (self.manifest_file) |file| { + if (builtin.os.tag == .windows) { + // See Lock.release for why this is required on Windows + file.unlock(); + } + file.close(); } for (self.files.items) |*file| { diff --git a/src/Compilation.zig b/src/Compilation.zig index e3c45678e2..f568753fea 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -3565,6 +3565,7 @@ pub fn cImport(comp: *Compilation, c_src: []const u8) !CImportResult { const cimport_zig_basename = "cimport.zig"; var man = comp.obtainCObjectCacheManifest(); + man.want_shared_lock = false; defer man.deinit(); man.hash.add(@as(u16, 0xb945)); // Random number to distinguish translate-c from compiling C objects @@ -3678,6 +3679,7 @@ pub fn cImport(comp: *Compilation, c_src: []const u8) !CImportResult { // possible we had a hit and the manifest is dirty, for example if the file mtime changed but // the contents were the same, we hit the cache but the manifest is dirty and we need to update // it to prevent doing a full file content comparison the next time around. + man.want_shared_lock = true; man.writeManifest() catch |err| { log.warn("failed to write cache manifest for C import: {s}", .{@errorName(err)}); }; @@ -3852,6 +3854,7 @@ fn updateCObject(comp: *Compilation, c_object: *CObject, c_obj_prog_node: *std.P } var man = comp.obtainCObjectCacheManifest(); + man.want_shared_lock = false; defer man.deinit(); man.hash.add(comp.clang_preprocessor_mode); @@ -4147,6 +4150,7 @@ fn updateCObject(comp: *Compilation, c_object: *CObject, c_obj_prog_node: *std.P // possible we had a hit and the manifest is dirty, for example if the file mtime changed but // the contents were the same, we hit the cache but the manifest is dirty and we need to update // it to prevent doing a full file content comparison the next time around. + man.want_shared_lock = true; man.writeManifest() catch |err| { log.warn("failed to write cache manifest when compiling '{s}': {s}", .{ c_object.src.src_path, @errorName(err) }); }; diff --git a/src/main.zig b/src/main.zig index 067a824631..7004576f46 100644 --- a/src/main.zig +++ b/src/main.zig @@ -3505,6 +3505,7 @@ fn cmdTranslateC(comp: *Compilation, arena: Allocator, enable_cache: bool) !void const translated_zig_basename = try std.fmt.allocPrint(arena, "{s}.zig", .{comp.bin_file.options.root_name}); var man: Cache.Manifest = comp.obtainCObjectCacheManifest(); + man.want_shared_lock = false; defer if (enable_cache) man.deinit(); man.hash.add(@as(u16, 0xb945)); // Random number to distinguish translate-c from compiling C objects |
