authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2020-12-25 19:02:15-07:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2020-12-25 19:02:15-07:00
loged39ff202baf5bb73e54f7ecc20df63d9c190dc9
tree140eec25db6ef3d22599ce4f0d65d46d59903f82
parentc452bb13220b11242358f06ba80487d4148dbb27

stage2: Cache: fix resource management of the deadlock debug code

1 files changed, 34 insertions(+), 7 deletions(-)

src/Cache.zig+34-7
...@@ -19,6 +19,7 @@ var all_cache_digest_set: std.AutoHashMapUnmanaged(BinDigest, void) = .{};...@@ -19,6 +19,7 @@ var all_cache_digest_set: std.AutoHashMapUnmanaged(BinDigest, void) = .{};
19var all_cache_digest_lock: std.Mutex = .{};19var all_cache_digest_lock: std.Mutex = .{};
20const want_debug_deadlock = true; // TODO change this for release builds20const want_debug_deadlock = true; // TODO change this for release builds
21const DebugBinDigest = if (want_debug_deadlock) BinDigest else void;21const DebugBinDigest = if (want_debug_deadlock) BinDigest else void;
22const null_debug_bin_digest = if (want_debug_deadlock) ([1]u8{0} ** bin_digest_len) else {};
2223
23/// Be sure to call `Manifest.deinit` after successful initialization.24/// Be sure to call `Manifest.deinit` after successful initialization.
24pub fn obtain(cache: *const Cache) Manifest {25pub fn obtain(cache: *const Cache) Manifest {
...@@ -193,7 +194,7 @@ pub const Manifest = struct {...@@ -193,7 +194,7 @@ pub const Manifest = struct {
193 manifest_dirty: bool,194 manifest_dirty: bool,
194 files: std.ArrayListUnmanaged(File) = .{},195 files: std.ArrayListUnmanaged(File) = .{},
195 hex_digest: [hex_digest_len]u8,196 hex_digest: [hex_digest_len]u8,
196 bin_digest: DebugBinDigest = undefined,197 debug_bin_digest: DebugBinDigest = null_debug_bin_digest,
197198
198 /// Add a file as a dependency of process being cached. When `hit` is199 /// Add a file as a dependency of process being cached. When `hit` is
199 /// called, the file's contents will be checked to ensure that it matches200 /// called, the file's contents will be checked to ensure that it matches
...@@ -262,7 +263,7 @@ pub const Manifest = struct {...@@ -262,7 +263,7 @@ pub const Manifest = struct {
262 self.hash.hasher.final(&bin_digest);263 self.hash.hasher.final(&bin_digest);
263264
264 if (want_debug_deadlock) {265 if (want_debug_deadlock) {
265 self.bin_digest = bin_digest;266 self.debug_bin_digest = bin_digest;
266267
267 const held = all_cache_digest_lock.acquire();268 const held = all_cache_digest_lock.acquire();
268 defer held.release();269 defer held.release();
...@@ -603,18 +604,27 @@ pub const Manifest = struct {...@@ -603,18 +604,27 @@ pub const Manifest = struct {
603 /// The `Manifest` remains safe to deinit.604 /// The `Manifest` remains safe to deinit.
604 /// Don't forget to call `writeManifest` before this!605 /// Don't forget to call `writeManifest` before this!
605 pub fn toOwnedLock(self: *Manifest) Lock {606 pub fn toOwnedLock(self: *Manifest) Lock {
606 const manifest_file = self.manifest_file.?;607 const lock: Lock = .{
607 self.manifest_file = null;608 .manifest_file = self.manifest_file.?,
608 return Lock{609 .debug_bin_digest = self.debug_bin_digest,
609 .manifest_file = manifest_file,
610 .debug_bin_digest = self.bin_digest,
611 };610 };
611 self.manifest_file = null;
612 self.debug_bin_digest = null_debug_bin_digest;
613 return lock;
612 }614 }
613615
614 /// Releases the manifest file and frees any memory the Manifest was using.616 /// Releases the manifest file and frees any memory the Manifest was using.
615 /// `Manifest.hit` must be called first.617 /// `Manifest.hit` must be called first.
616 /// Don't forget to call `writeManifest` before this!618 /// Don't forget to call `writeManifest` before this!
617 pub fn deinit(self: *Manifest) void {619 pub fn deinit(self: *Manifest) void {
620 if (want_debug_deadlock) {
621 if (!mem.eql(u8, &self.debug_bin_digest, &null_debug_bin_digest)) {
622 const held = all_cache_digest_lock.acquire();
623 defer held.release();
624
625 all_cache_digest_set.removeAssertDiscard(self.debug_bin_digest);
626 }
627 }
618 if (self.manifest_file) |file| {628 if (self.manifest_file) |file| {
619 file.close();629 file.close();
620 }630 }
...@@ -698,6 +708,11 @@ test "cache file and then recall it" {...@@ -698,6 +708,11 @@ test "cache file and then recall it" {
698 // https://github.com/ziglang/zig/issues/5437708 // https://github.com/ziglang/zig/issues/5437
699 return error.SkipZigTest;709 return error.SkipZigTest;
700 }710 }
711 defer if (want_debug_deadlock) {
712 testing.expect(all_cache_digest_set.count() == 0);
713 all_cache_digest_set.clearAndFree(testing.allocator);
714 };
715
701 const cwd = fs.cwd();716 const cwd = fs.cwd();
702717
703 const temp_file = "test.txt";718 const temp_file = "test.txt";
...@@ -775,6 +790,10 @@ test "check that changing a file makes cache fail" {...@@ -775,6 +790,10 @@ test "check that changing a file makes cache fail" {
775 // https://github.com/ziglang/zig/issues/5437790 // https://github.com/ziglang/zig/issues/5437
776 return error.SkipZigTest;791 return error.SkipZigTest;
777 }792 }
793 defer if (want_debug_deadlock) {
794 testing.expect(all_cache_digest_set.count() == 0);
795 all_cache_digest_set.clearAndFree(testing.allocator);
796 };
778 const cwd = fs.cwd();797 const cwd = fs.cwd();
779798
780 const temp_file = "cache_hash_change_file_test.txt";799 const temp_file = "cache_hash_change_file_test.txt";
...@@ -851,6 +870,10 @@ test "no file inputs" {...@@ -851,6 +870,10 @@ test "no file inputs" {
851 // https://github.com/ziglang/zig/issues/5437870 // https://github.com/ziglang/zig/issues/5437
852 return error.SkipZigTest;871 return error.SkipZigTest;
853 }872 }
873 defer if (want_debug_deadlock) {
874 testing.expect(all_cache_digest_set.count() == 0);
875 all_cache_digest_set.clearAndFree(testing.allocator);
876 };
854 const cwd = fs.cwd();877 const cwd = fs.cwd();
855 const temp_manifest_dir = "no_file_inputs_manifest_dir";878 const temp_manifest_dir = "no_file_inputs_manifest_dir";
856 defer cwd.deleteTree(temp_manifest_dir) catch {};879 defer cwd.deleteTree(temp_manifest_dir) catch {};
...@@ -896,6 +919,10 @@ test "Manifest with files added after initial hash work" {...@@ -896,6 +919,10 @@ test "Manifest with files added after initial hash work" {
896 // https://github.com/ziglang/zig/issues/5437919 // https://github.com/ziglang/zig/issues/5437
897 return error.SkipZigTest;920 return error.SkipZigTest;
898 }921 }
922 defer if (want_debug_deadlock) {
923 testing.expect(all_cache_digest_set.count() == 0);
924 all_cache_digest_set.clearAndFree(testing.allocator);
925 };
899 const cwd = fs.cwd();926 const cwd = fs.cwd();
900927
901 const temp_file1 = "cache_hash_post_file_test1.txt";928 const temp_file1 = "cache_hash_post_file_test1.txt";