From c7834f274d9c48e1ef155a367cbb598c239b8c7a Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 25 Dec 2020 18:38:49 -0700 Subject: [PATCH 1/6] stage2: Cache: add debug deadlock detection code --- src/Cache.zig | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/src/Cache.zig b/src/Cache.zig index 856d2c4277..c145f08cf2 100644 --- a/src/Cache.zig +++ b/src/Cache.zig @@ -12,6 +12,14 @@ 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. +/// The plan is to enable this for debug builds only, but for now we enable +/// it always to catch a deadlock. +var all_cache_digest_set: std.AutoHashMapUnmanaged(BinDigest, void) = .{}; +var all_cache_digest_lock: std.Mutex = .{}; +const want_debug_deadlock = true; // TODO change this for release builds +const DebugBinDigest = if (want_debug_deadlock) BinDigest else void; + /// Be sure to call `Manifest.deinit` after successful initialization. pub fn obtain(cache: *const Cache) Manifest { return Manifest{ @@ -160,8 +168,15 @@ 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; } @@ -178,6 +193,7 @@ pub const Manifest = struct { manifest_dirty: bool, files: std.ArrayListUnmanaged(File) = .{}, hex_digest: [hex_digest_len]u8, + bin_digest: DebugBinDigest = undefined, /// Add a file as a dependency of process being cached. When `hit` is /// called, the file's contents will be checked to ensure that it matches @@ -245,6 +261,23 @@ pub const Manifest = struct { var bin_digest: BinDigest = undefined; self.hash.hasher.final(&bin_digest); + if (want_debug_deadlock) { + self.bin_digest = bin_digest; + + const held = all_cache_digest_lock.acquire(); + defer held.release(); + + 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; @@ -572,7 +605,10 @@ pub const Manifest = struct { pub fn toOwnedLock(self: *Manifest) Lock { const manifest_file = self.manifest_file.?; self.manifest_file = null; - return Lock{ .manifest_file = manifest_file }; + return Lock{ + .manifest_file = manifest_file, + .debug_bin_digest = self.bin_digest, + }; } /// Releases the manifest file and frees any memory the Manifest was using. From c452bb13220b11242358f06ba80487d4148dbb27 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 25 Dec 2020 18:39:13 -0700 Subject: [PATCH 2/6] ci: build in Debug mode to help find the deadlock --- ci/drone/linux_script | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/drone/linux_script b/ci/drone/linux_script index 8c5dc1be2a..45557fd996 100755 --- a/ci/drone/linux_script +++ b/ci/drone/linux_script @@ -17,8 +17,8 @@ git config core.abbrev 9 mkdir build cd build -# TODO figure out why Drone CI is deadlocking and stop passing -DZIG_SINGLE_THREADED=ON -cmake .. -DCMAKE_BUILD_TYPE=Release "-DCMAKE_INSTALL_PREFIX=$DISTDIR" -DZIG_STATIC=ON -DCMAKE_PREFIX_PATH=/deps/local -GNinja -DZIG_SINGLE_THREADED=ON +# TODO put this back on Release +cmake .. -DCMAKE_BUILD_TYPE=Debug "-DCMAKE_INSTALL_PREFIX=$DISTDIR" -DZIG_STATIC=ON -DCMAKE_PREFIX_PATH=/deps/local -GNinja samu install ./zig build test -Dskip-release -Dskip-non-native From ed39ff202baf5bb73e54f7ecc20df63d9c190dc9 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 25 Dec 2020 19:02:15 -0700 Subject: [PATCH 3/6] stage2: Cache: fix resource management of the deadlock debug code --- src/Cache.zig | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/src/Cache.zig b/src/Cache.zig index c145f08cf2..9f32f15a01 100644 --- a/src/Cache.zig +++ b/src/Cache.zig @@ -19,6 +19,7 @@ var all_cache_digest_set: std.AutoHashMapUnmanaged(BinDigest, void) = .{}; var all_cache_digest_lock: std.Mutex = .{}; const want_debug_deadlock = true; // TODO change this for release builds 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 { @@ -193,7 +194,7 @@ pub const Manifest = struct { manifest_dirty: bool, files: std.ArrayListUnmanaged(File) = .{}, hex_digest: [hex_digest_len]u8, - bin_digest: DebugBinDigest = undefined, + debug_bin_digest: DebugBinDigest = null_debug_bin_digest, /// Add a file as a dependency of process being cached. When `hit` is /// called, the file's contents will be checked to ensure that it matches @@ -262,7 +263,7 @@ pub const Manifest = struct { self.hash.hasher.final(&bin_digest); if (want_debug_deadlock) { - self.bin_digest = bin_digest; + self.debug_bin_digest = bin_digest; const held = all_cache_digest_lock.acquire(); defer held.release(); @@ -603,18 +604,27 @@ pub const Manifest = struct { /// The `Manifest` remains safe to deinit. /// Don't forget to call `writeManifest` before this! pub fn toOwnedLock(self: *Manifest) Lock { - const manifest_file = self.manifest_file.?; - self.manifest_file = null; - return Lock{ - .manifest_file = manifest_file, - .debug_bin_digest = self.bin_digest, + 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(); } @@ -698,6 +708,11 @@ test "cache file and then recall it" { // https://github.com/ziglang/zig/issues/5437 return error.SkipZigTest; } + defer if (want_debug_deadlock) { + testing.expect(all_cache_digest_set.count() == 0); + all_cache_digest_set.clearAndFree(testing.allocator); + }; + const cwd = fs.cwd(); const temp_file = "test.txt"; @@ -775,6 +790,10 @@ test "check that changing a file makes cache fail" { // https://github.com/ziglang/zig/issues/5437 return error.SkipZigTest; } + defer if (want_debug_deadlock) { + testing.expect(all_cache_digest_set.count() == 0); + all_cache_digest_set.clearAndFree(testing.allocator); + }; const cwd = fs.cwd(); const temp_file = "cache_hash_change_file_test.txt"; @@ -851,6 +870,10 @@ test "no file inputs" { // https://github.com/ziglang/zig/issues/5437 return error.SkipZigTest; } + defer if (want_debug_deadlock) { + testing.expect(all_cache_digest_set.count() == 0); + all_cache_digest_set.clearAndFree(testing.allocator); + }; const cwd = fs.cwd(); const temp_manifest_dir = "no_file_inputs_manifest_dir"; defer cwd.deleteTree(temp_manifest_dir) catch {}; @@ -896,6 +919,10 @@ test "Manifest with files added after initial hash work" { // https://github.com/ziglang/zig/issues/5437 return error.SkipZigTest; } + defer if (want_debug_deadlock) { + testing.expect(all_cache_digest_set.count() == 0); + all_cache_digest_set.clearAndFree(testing.allocator); + }; const cwd = fs.cwd(); const temp_file1 = "cache_hash_post_file_test1.txt"; From 3f9588ca29c721f11eb9dae8f6de8ebd1155c7bf Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sat, 26 Dec 2020 13:50:26 -0700 Subject: [PATCH 4/6] std: do not call malloc() between fork() and execv() We were violating the POSIX standard which resulted in a deadlock on musl v1.1.24 on aarch64 alpine linux, uncovered with the new ThreadPool usage in the stage2 compiler. std.os execv functions that accept an Allocator parameter are removed because they are footguns. The POSIX standard does not allow calls to malloc() between fork() and execv() and since it is common to both (1) call execv() after fork() and (2) use std.heap.c_allocator, Programmers are encouraged to go through the `std.process` API instead, causing some dissonance when combined with `std.os` APIs. I also slapped a big warning message on all the relevant doc comments. --- lib/std/child_process.zig | 76 +++++++++++++++++++++++++++++------ lib/std/os.zig | 83 +-------------------------------------- lib/std/process.zig | 66 +++++++++++++++++++++++++++++++ src/main.zig | 14 +++---- 4 files changed, 136 insertions(+), 103 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index 6f75b01c87..eb6bf81146 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -19,6 +19,7 @@ const builtin = @import("builtin"); const Os = builtin.Os; const TailQueue = std.TailQueue; const maxInt = std.math.maxInt; +const assert = std.debug.assert; pub const ChildProcess = struct { pid: if (builtin.os.tag == .windows) void else i32, @@ -376,19 +377,44 @@ pub const ChildProcess = struct { if (any_ignore) os.close(dev_null_fd); } - var env_map_owned: BufMap = undefined; - var we_own_env_map: bool = undefined; - const env_map = if (self.env_map) |env_map| x: { - we_own_env_map = false; - break :x env_map; - } else x: { - we_own_env_map = true; - env_map_owned = try process.getEnvMap(self.allocator); - break :x &env_map_owned; - }; - defer { - if (we_own_env_map) env_map_owned.deinit(); + var arena_allocator = std.heap.ArenaAllocator.init(self.allocator); + defer arena_allocator.deinit(); + const arena = &arena_allocator.allocator; + + // The POSIX standard does not allow malloc() between fork() and execve(), + // and `self.allocator` may be a libc allocator. + // I have personally observed the child process deadlocking when it tries + // to call malloc() due to a heap allocation between fork() and execve(), + // in musl v1.1.24. + // Additionally, we want to reduce the number of possible ways things + // can fail between fork() and execve(). + // Therefore, we do all the allocation for the execve() before the fork(). + // This means we must do the null-termination of argv and env vars here. + const argv_buf = try arena.alloc(?[*:0]u8, self.argv.len + 1); + for (self.argv) |arg, i| { + const arg_buf = try arena.alloc(u8, arg.len + 1); + @memcpy(arg_buf.ptr, arg.ptr, arg.len); + arg_buf[arg.len] = 0; + argv_buf[i] = arg_buf[0..arg.len :0].ptr; } + argv_buf[self.argv.len] = null; + const argv_ptr = argv_buf[0..self.argv.len :null].ptr; + + const envp = m: { + if (self.env_map) |env_map| { + const envp_buf = try createNullDelimitedEnvMap(arena, env_map); + break :m envp_buf.ptr; + } else if (std.builtin.link_libc) { + break :m std.c.environ; + } else if (std.builtin.output_mode == .Exe) { + // Then we have Zig start code and this works. + // TODO type-safety for null-termination of `os.environ`. + break :m @ptrCast([*:null]?[*:0]u8, os.environ.ptr); + } else { + // TODO come up with a solution for this. + @compileError("missing std lib enhancement: ChildProcess implementation has no way to collect the environment variables to forward to the child process"); + } + }; // This pipe is used to communicate errors between the time of fork // and execve from the child process to the parent process. @@ -438,7 +464,10 @@ pub const ChildProcess = struct { os.setreuid(uid, uid) catch |err| forkChildErrReport(err_pipe[1], err); } - const err = os.execvpe_expandArg0(self.allocator, self.expand_arg0, self.argv, env_map); + const err = switch (self.expand_arg0) { + .expand => os.execvpeZ_expandArg0(.expand, argv_buf.ptr[0].?, argv_ptr, envp), + .no_expand => os.execvpeZ_expandArg0(.no_expand, argv_buf.ptr[0].?, argv_ptr, envp), + }; forkChildErrReport(err_pipe[1], err); } @@ -881,3 +910,24 @@ pub fn createWindowsEnvBlock(allocator: *mem.Allocator, env_map: *const BufMap) i += 1; return allocator.shrink(result, i); } + +pub fn createNullDelimitedEnvMap(arena: *mem.Allocator, env_map: *const std.BufMap) ![:null]?[*:0]u8 { + const envp_count = env_map.count(); + const envp_buf = try arena.alloc(?[*:0]u8, envp_count + 1); + mem.set(?[*:0]u8, envp_buf, null); + { + var it = env_map.iterator(); + var i: usize = 0; + while (it.next()) |pair| : (i += 1) { + const env_buf = try arena.alloc(u8, pair.key.len + pair.value.len + 2); + @memcpy(env_buf.ptr, pair.key.ptr, pair.key.len); + env_buf[pair.key.len] = '='; + @memcpy(env_buf.ptr + pair.key.len + 1, pair.value.ptr, pair.value.len); + const len = env_buf.len - 1; + env_buf[len] = 0; + envp_buf[i] = env_buf[0..len :0].ptr; + } + assert(i == envp_count); + } + return envp_buf[0..envp_count :null]; +} diff --git a/lib/std/os.zig b/lib/std/os.zig index 5a57fed5cf..28d4f6bb1a 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -1348,89 +1348,10 @@ pub fn execvpeZ_expandArg0( /// If `file` is an absolute path, this is the same as `execveZ`. pub fn execvpeZ( file: [*:0]const u8, - argv: [*:null]const ?[*:0]const u8, + argv_ptr: [*:null]const ?[*:0]const u8, envp: [*:null]const ?[*:0]const u8, ) ExecveError { - return execvpeZ_expandArg0(.no_expand, file, argv, envp); -} - -/// This is the same as `execvpe` except if the `arg0_expand` parameter is set to `.expand`, -/// then argv[0] will be replaced with the expanded version of it, after resolving in accordance -/// with the PATH environment variable. -pub fn execvpe_expandArg0( - allocator: *mem.Allocator, - arg0_expand: Arg0Expand, - argv_slice: []const []const u8, - env_map: *const std.BufMap, -) (ExecveError || error{OutOfMemory}) { - const argv_buf = try allocator.alloc(?[*:0]u8, argv_slice.len + 1); - mem.set(?[*:0]u8, argv_buf, null); - defer { - for (argv_buf) |arg| { - const arg_buf = mem.spanZ(arg) orelse break; - allocator.free(arg_buf); - } - allocator.free(argv_buf); - } - for (argv_slice) |arg, i| { - const arg_buf = try allocator.alloc(u8, arg.len + 1); - @memcpy(arg_buf.ptr, arg.ptr, arg.len); - arg_buf[arg.len] = 0; - argv_buf[i] = arg_buf[0..arg.len :0].ptr; - } - argv_buf[argv_slice.len] = null; - const argv_ptr = argv_buf[0..argv_slice.len :null].ptr; - - const envp_buf = try createNullDelimitedEnvMap(allocator, env_map); - defer freeNullDelimitedEnvMap(allocator, envp_buf); - - switch (arg0_expand) { - .expand => return execvpeZ_expandArg0(.expand, argv_buf.ptr[0].?, argv_ptr, envp_buf.ptr), - .no_expand => return execvpeZ_expandArg0(.no_expand, argv_buf.ptr[0].?, argv_ptr, envp_buf.ptr), - } -} - -/// This function must allocate memory to add a null terminating bytes on path and each arg. -/// It must also convert to KEY=VALUE\0 format for environment variables, and include null -/// pointers after the args and after the environment variables. -/// `argv_slice[0]` is the executable path. -/// This function also uses the PATH environment variable to get the full path to the executable. -pub fn execvpe( - allocator: *mem.Allocator, - argv_slice: []const []const u8, - env_map: *const std.BufMap, -) (ExecveError || error{OutOfMemory}) { - return execvpe_expandArg0(allocator, .no_expand, argv_slice, env_map); -} - -pub fn createNullDelimitedEnvMap(allocator: *mem.Allocator, env_map: *const std.BufMap) ![:null]?[*:0]u8 { - const envp_count = env_map.count(); - const envp_buf = try allocator.alloc(?[*:0]u8, envp_count + 1); - mem.set(?[*:0]u8, envp_buf, null); - errdefer freeNullDelimitedEnvMap(allocator, envp_buf); - { - var it = env_map.iterator(); - var i: usize = 0; - while (it.next()) |pair| : (i += 1) { - const env_buf = try allocator.alloc(u8, pair.key.len + pair.value.len + 2); - @memcpy(env_buf.ptr, pair.key.ptr, pair.key.len); - env_buf[pair.key.len] = '='; - @memcpy(env_buf.ptr + pair.key.len + 1, pair.value.ptr, pair.value.len); - const len = env_buf.len - 1; - env_buf[len] = 0; - envp_buf[i] = env_buf[0..len :0].ptr; - } - assert(i == envp_count); - } - return envp_buf[0..envp_count :null]; -} - -pub fn freeNullDelimitedEnvMap(allocator: *mem.Allocator, envp_buf: []?[*:0]u8) void { - for (envp_buf) |env| { - const env_buf = if (env) |ptr| ptr[0 .. mem.len(ptr) + 1] else break; - allocator.free(env_buf); - } - allocator.free(envp_buf); + return execvpeZ_expandArg0(.no_expand, file, argv_ptr, envp); } /// Get an environment variable. diff --git a/lib/std/process.zig b/lib/std/process.zig index e12bc28c0c..909b337f89 100644 --- a/lib/std/process.zig +++ b/lib/std/process.zig @@ -13,6 +13,7 @@ const math = std.math; const Allocator = mem.Allocator; const assert = std.debug.assert; const testing = std.testing; +const child_process = @import("child_process.zig"); pub const abort = os.abort; pub const exit = os.exit; @@ -778,3 +779,68 @@ pub fn getSelfExeSharedLibPaths(allocator: *Allocator) error{OutOfMemory}![][:0] else => @compileError("getSelfExeSharedLibPaths unimplemented for this target"), } } + +/// Tells whether calling the `execv` or `execve` functions will be a compile error. +pub const can_execv = std.builtin.os.tag != .windows; + +pub const ExecvError = std.os.ExecveError || error{OutOfMemory}; + +/// Replaces the current process image with the executed process. +/// This function must allocate memory to add a null terminating bytes on path and each arg. +/// It must also convert to KEY=VALUE\0 format for environment variables, and include null +/// pointers after the args and after the environment variables. +/// `argv[0]` is the executable path. +/// This function also uses the PATH environment variable to get the full path to the executable. +/// Due to the heap-allocation, it is illegal to call this function in a fork() child. +/// For that use case, use the `std.os` functions directly. +pub fn execv(allocator: *mem.Allocator, argv: []const []const u8) ExecvError { + return execve(allocator, argv, null); +} + +/// Replaces the current process image with the executed process. +/// This function must allocate memory to add a null terminating bytes on path and each arg. +/// It must also convert to KEY=VALUE\0 format for environment variables, and include null +/// pointers after the args and after the environment variables. +/// `argv[0]` is the executable path. +/// This function also uses the PATH environment variable to get the full path to the executable. +/// Due to the heap-allocation, it is illegal to call this function in a fork() child. +/// For that use case, use the `std.os` functions directly. +pub fn execve( + allocator: *mem.Allocator, + argv: []const []const u8, + env_map: ?*const std.BufMap, +) ExecvError { + if (!can_execv) @compileError("The target OS does not support execv"); + + var arena_allocator = std.heap.ArenaAllocator.init(allocator); + defer arena_allocator.deinit(); + const arena = &arena_allocator.allocator; + + const argv_buf = try arena.alloc(?[*:0]u8, argv.len + 1); + for (argv) |arg, i| { + const arg_buf = try arena.alloc(u8, arg.len + 1); + @memcpy(arg_buf.ptr, arg.ptr, arg.len); + arg_buf[arg.len] = 0; + argv_buf[i] = arg_buf[0..arg.len :0].ptr; + } + argv_buf[argv.len] = null; + const argv_ptr = argv_buf[0..argv.len :null].ptr; + + const envp = m: { + if (env_map) |m| { + const envp_buf = try child_process.createNullDelimitedEnvMap(arena, m); + break :m envp_buf.ptr; + } else if (std.builtin.link_libc) { + break :m std.c.environ; + } else if (std.builtin.output_mode == .Exe) { + // Then we have Zig start code and this works. + // TODO type-safety for null-termination of `os.environ`. + break :m @ptrCast([*:null]?[*:0]u8, os.environ.ptr); + } else { + // TODO come up with a solution for this. + @compileError("missing std lib enhancement: std.process.execv implementation has no way to collect the environment variables to forward to the child process"); + } + }; + + return os.execvpeZ_expandArg0(.no_expand, argv_buf.ptr[0].?, argv_ptr, envp); +} diff --git a/src/main.zig b/src/main.zig index 99d0dcd5f9..c23cba98d8 100644 --- a/src/main.zig +++ b/src/main.zig @@ -116,15 +116,13 @@ pub fn main() anyerror!void { return mainArgs(gpa, arena, args); } -const os_can_execve = std.builtin.os.tag != .windows; - pub fn mainArgs(gpa: *Allocator, arena: *Allocator, args: []const []const u8) !void { if (args.len <= 1) { std.log.info("{}", .{usage}); fatal("expected command argument", .{}); } - if (os_can_execve and std.os.getenvZ("ZIG_IS_DETECTING_LIBC_PATHS") != null) { + if (std.process.can_execv and std.os.getenvZ("ZIG_IS_DETECTING_LIBC_PATHS") != null) { // In this case we have accidentally invoked ourselves as "the system C compiler" // to figure out where libc is installed. This is essentially infinite recursion // via child process execution due to the CC environment variable pointing to Zig. @@ -147,11 +145,11 @@ pub fn mainArgs(gpa: *Allocator, arena: *Allocator, args: []const []const u8) !v // CC environment variable. We detect and support this scenario here because of // the ZIG_IS_DETECTING_LIBC_PATHS environment variable. if (mem.eql(u8, args[1], "cc")) { - return std.os.execvpe(arena, args[1..], &env_map); + return std.process.execve(arena, args[1..], &env_map); } else { const modified_args = try arena.dupe([]const u8, args); modified_args[0] = "cc"; - return std.os.execvpe(arena, modified_args, &env_map); + return std.process.execve(arena, modified_args, &env_map); } } @@ -1841,10 +1839,8 @@ fn buildOutputType( } // We do not execve for tests because if the test fails we want to print the error message and // invocation below. - if (os_can_execve and arg_mode == .run and !watch) { - // TODO improve the std lib so that we don't need a call to getEnvMap here. - var env_vars = try process.getEnvMap(arena); - const err = std.os.execvpe(gpa, argv.items, &env_vars); + if (std.process.can_execv and arg_mode == .run and !watch) { + const err = std.process.execv(gpa, argv.items); const cmd = try argvCmd(arena, argv.items); fatal("the following command failed to execve with '{s}':\n{s}", .{ @errorName(err), cmd }); } else { From 3366e4caf3ab3f85cfac9b57b84db4eb9ee0a078 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sat, 26 Dec 2020 13:55:38 -0700 Subject: [PATCH 5/6] ci: put Drone CI back to normal Surely this time all the problems have been fixed --- ci/drone/linux_script | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ci/drone/linux_script b/ci/drone/linux_script index 45557fd996..fdc1704fb7 100755 --- a/ci/drone/linux_script +++ b/ci/drone/linux_script @@ -17,8 +17,7 @@ git config core.abbrev 9 mkdir build cd build -# TODO put this back on Release -cmake .. -DCMAKE_BUILD_TYPE=Debug "-DCMAKE_INSTALL_PREFIX=$DISTDIR" -DZIG_STATIC=ON -DCMAKE_PREFIX_PATH=/deps/local -GNinja +cmake .. -DCMAKE_BUILD_TYPE=Release "-DCMAKE_INSTALL_PREFIX=$DISTDIR" -DZIG_STATIC=ON -DCMAKE_PREFIX_PATH=/deps/local -GNinja samu install ./zig build test -Dskip-release -Dskip-non-native From cb290ed6c99681ade0bace286ae5040546542395 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sat, 26 Dec 2020 14:02:07 -0700 Subject: [PATCH 6/6] stage2: Cache deadlock debugging only for safe build modes --- src/Cache.zig | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Cache.zig b/src/Cache.zig index 9f32f15a01..7d77782a6f 100644 --- a/src/Cache.zig +++ b/src/Cache.zig @@ -13,11 +13,10 @@ const fmt = std.fmt; const Allocator = std.mem.Allocator; /// Process-scoped map keeping track of all locked Cache hashes, to detect deadlocks. -/// The plan is to enable this for debug builds only, but for now we enable -/// it always to catch a deadlock. +/// 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 = .{}; -const want_debug_deadlock = true; // TODO change this for release builds +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 {};