Skip to content

Commit

Permalink
stage2: remove the Cache deadlock detection code
Browse files Browse the repository at this point in the history
It's more trouble than it's worth; it didn't even catch the most recent
incident because it was across process boundaries anyway.
  • Loading branch information
andrewrk committed Jan 4, 2021
1 parent 404dc96 commit c8e44d8
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 68 deletions.
66 changes: 0 additions & 66 deletions src/Cache.zig
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,6 @@ const mem = std.mem;
const fmt = std.fmt;
const Allocator = std.mem.Allocator;

/// Process-scoped map keeping track of all locked Cache hashes, to detect deadlocks.
/// This protection is conditionally compiled depending on `want_debug_deadlock`.
var all_cache_digest_set: std.AutoHashMapUnmanaged(BinDigest, void) = .{};
var all_cache_digest_lock: std.Mutex = .{};
var all_cache_digest_allocator: ?*Allocator = null;
const want_debug_deadlock = std.debug.runtime_safety;
const DebugBinDigest = if (want_debug_deadlock) BinDigest else void;
const null_debug_bin_digest = if (want_debug_deadlock) ([1]u8{0} ** bin_digest_len) else {};

/// Be sure to call `Manifest.deinit` after successful initialization.
pub fn obtain(cache: *const Cache) Manifest {
return Manifest{
Expand Down Expand Up @@ -169,15 +160,8 @@ pub const HashHelper = struct {

pub const Lock = struct {
manifest_file: fs.File,
debug_bin_digest: DebugBinDigest,

pub fn release(lock: *Lock) void {
if (want_debug_deadlock) {
const held = all_cache_digest_lock.acquire();
defer held.release();

all_cache_digest_set.removeAssertDiscard(lock.debug_bin_digest);
}
lock.manifest_file.close();
lock.* = undefined;
}
Expand All @@ -194,7 +178,6 @@ pub const Manifest = struct {
manifest_dirty: bool,
files: std.ArrayListUnmanaged(File) = .{},
hex_digest: [hex_digest_len]u8,
debug_bin_digest: DebugBinDigest = null_debug_bin_digest,
/// Populated when hit() returns an error because of one
/// of the files listed in the manifest.
failed_file_index: ?usize = null,
Expand Down Expand Up @@ -267,31 +250,6 @@ pub const Manifest = struct {
var bin_digest: BinDigest = undefined;
self.hash.hasher.final(&bin_digest);

if (want_debug_deadlock) {
self.debug_bin_digest = bin_digest;

const held = all_cache_digest_lock.acquire();
defer held.release();

if (all_cache_digest_allocator) |prev_gpa| {
if (prev_gpa != self.cache.gpa) {
@panic("The deadlock debug code in Cache depends on using the same allocator for everything");
}
} else {
all_cache_digest_allocator = self.cache.gpa;
}

const gop = try all_cache_digest_set.getOrPut(self.cache.gpa, bin_digest);
if (gop.found_existing) {
std.debug.print("Cache deadlock detected in Cache.hit. Manifest has {d} files:\n", .{self.files.items.len});
for (self.files.items) |file| {
const p: []const u8 = file.path orelse "(null)";
std.debug.print(" file: {s}\n", .{p});
}
@panic("Cache deadlock detected");
}
}

_ = std.fmt.bufPrint(&self.hex_digest, "{x}", .{bin_digest}) catch unreachable;

self.hash.hasher = hasher_init;
Expand Down Expand Up @@ -628,25 +586,15 @@ pub const Manifest = struct {
pub fn toOwnedLock(self: *Manifest) Lock {
const lock: Lock = .{
.manifest_file = self.manifest_file.?,
.debug_bin_digest = self.debug_bin_digest,
};
self.manifest_file = null;
self.debug_bin_digest = null_debug_bin_digest;
return lock;
}

/// Releases the manifest file and frees any memory the Manifest was using.
/// `Manifest.hit` must be called first.
/// Don't forget to call `writeManifest` before this!
pub fn deinit(self: *Manifest) void {
if (want_debug_deadlock) {
if (!mem.eql(u8, &self.debug_bin_digest, &null_debug_bin_digest)) {
const held = all_cache_digest_lock.acquire();
defer held.release();

all_cache_digest_set.removeAssertDiscard(self.debug_bin_digest);
}
}
if (self.manifest_file) |file| {
file.close();
}
Expand Down Expand Up @@ -725,22 +673,11 @@ fn isProblematicTimestamp(fs_clock: i128) bool {
return wall_nsec == fs_nsec and wall_sec == fs_sec;
}

pub fn deinitDebugMap() void {
if (!want_debug_deadlock) return;

if (all_cache_digest_set.count() != 0) {
@panic("there's a Cache not deinitialized somewhere");
}
const gpa = all_cache_digest_allocator orelse return;
all_cache_digest_set.clearAndFree(gpa);
}

test "cache file and then recall it" {
if (std.Target.current.os.tag == .wasi) {
// https://github.com/ziglang/zig/issues/5437
return error.SkipZigTest;
}
defer deinitDebugMap();

const cwd = fs.cwd();

Expand Down Expand Up @@ -819,7 +756,6 @@ test "check that changing a file makes cache fail" {
// https://github.com/ziglang/zig/issues/5437
return error.SkipZigTest;
}
defer deinitDebugMap();
const cwd = fs.cwd();

const temp_file = "cache_hash_change_file_test.txt";
Expand Down Expand Up @@ -896,7 +832,6 @@ test "no file inputs" {
// https://github.com/ziglang/zig/issues/5437
return error.SkipZigTest;
}
defer deinitDebugMap();
const cwd = fs.cwd();
const temp_manifest_dir = "no_file_inputs_manifest_dir";
defer cwd.deleteTree(temp_manifest_dir) catch {};
Expand Down Expand Up @@ -942,7 +877,6 @@ test "Manifest with files added after initial hash work" {
// https://github.com/ziglang/zig/issues/5437
return error.SkipZigTest;
}
defer deinitDebugMap();
const cwd = fs.cwd();

const temp_file1 = "cache_hash_post_file_test1.txt";
Expand Down
1 change: 0 additions & 1 deletion src/main.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3292,7 +3292,6 @@ fn detectNativeTargetInfo(gpa: *Allocator, cross_target: std.zig.CrossTarget) !s
/// calls exit(0), and does not return.
pub fn cleanExit() void {
if (std.builtin.mode == .Debug) {
Cache.deinitDebugMap();
return;
} else {
process.exit(0);
Expand Down
1 change: 0 additions & 1 deletion src/test.zig
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,6 @@ pub const TestContext = struct {
case.updates.deinit();
}
self.cases.deinit();
@import("Cache.zig").deinitDebugMap();
self.* = undefined;
}

Expand Down

0 comments on commit c8e44d8

Please sign in to comment.