Skip to content

Commit

Permalink
fix: check if we are crashing before exiting gracefully (#12865)
Browse files Browse the repository at this point in the history
  • Loading branch information
paperdave committed Jul 27, 2024
1 parent e5ac4f9 commit 70ca2b7
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 37 deletions.
13 changes: 7 additions & 6 deletions src/Global.zig
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,13 @@ pub fn isExiting() bool {
pub fn exit(code: u32) noreturn {
is_exiting.store(true, .monotonic);

if (comptime Environment.isMac) {
std.c.exit(@bitCast(code));
// If we are crashing, allow the crash handler to finish it's work.
bun.crash_handler.sleepForeverIfAnotherThreadIsCrashing();

switch (Environment.os) {
.mac => std.c.exit(@bitCast(code)),
else => bun.C.quick_exit(@bitCast(code)),
}
bun.C.quick_exit(@bitCast(code));
}

pub fn raiseIgnoringPanicHandler(sig: bun.SignalCode) noreturn {
Expand Down Expand Up @@ -152,11 +155,9 @@ pub inline fn configureAllocator(_: AllocatorConfiguration) void {
// if (!config.long_running) Mimalloc.mi_option_set(Mimalloc.mi_option_reset_delay, 0);
}

pub const panic = Output.panic; // deprecated

pub fn notimpl() noreturn {
@setCold(true);
Global.panic("Not implemented yet!!!!!", .{});
Output.panic("Not implemented yet!!!!!", .{});
}

// Make sure we always print any leftover
Expand Down
4 changes: 2 additions & 2 deletions src/bundler.zig
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ pub const Bundler = struct {
framework.resolved = true;
this.options.framework = framework.*;
} else if (!framework.resolved) {
Global.panic("directly passing framework path is not implemented yet!", .{});
Output.panic("directly passing framework path is not implemented yet!", .{});
}
}
}
Expand Down Expand Up @@ -1649,7 +1649,7 @@ pub const Bundler = struct {
}
},
.css => {},
else => Global.panic("Unsupported loader {s} for path: {s}", .{ @tagName(loader), source.path.text }),
else => Output.panic("Unsupported loader {s} for path: {s}", .{ @tagName(loader), source.path.text }),
}

return null;
Expand Down
37 changes: 31 additions & 6 deletions src/crash_handler.zig
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,6 @@ pub fn crashHandler(
) noreturn {
@setCold(true);

// If a segfault happens while panicking, we want it to actually segfault, not trigger
// the handler.
resetSegfaultHandler();

if (bun.Environment.isDebug)
bun.Output.disableScopedDebugWriter();

Expand All @@ -197,7 +193,7 @@ pub fn crashHandler(
const writer = std.io.getStdErr().writer();

// The format of the panic trace is slightly different in debug
// builds Mainly, we demangle the backtrace immediately instead
// builds. Mainly, we demangle the backtrace immediately instead
// of using a trace string.
//
// To make the release-mode behavior easier to demo, debug mode
Expand Down Expand Up @@ -346,12 +342,22 @@ pub fn crashHandler(
writer.writeAll("\n") catch std.posix.abort();
}
}

// Be aware that this function only lets one thread return from it.
// This is important so that we do not try to run the following reload logic twice.
waitForOtherThreadToFinishPanicking();

report(trace_str_buf.slice());

// At this point, the crash handler has performed it's job. Reset the segfault handler
// so that a crash will actually crash. We need this because we want the process to
// exit with a signal, and allow tools to be able to gather core dumps.
//
// This is done so late (in comparison to the Zig Standard Library's panic handler)
// because if multiple threads segfault (more often the case on Windows), we don't
// want another thread to interrupt the crashing of the first one.
resetSegfaultHandler();

if (bun.auto_reload_on_crash and
// Do not reload if the panic arose FROM the reload function.
!bun.isProcessReloadInProgressOnAnotherThread())
Expand All @@ -371,6 +377,8 @@ pub fn crashHandler(
inline 1, 2 => |t| {
if (t == 1) {
panic_stage = 2;

resetSegfaultHandler();
Output.flush();
}
panic_stage = 3;
Expand All @@ -384,6 +392,7 @@ pub fn crashHandler(
},
3 => {
// Panicked while printing "Panicked during a panic."
panic_stage = 4;
},
else => {
// Panicked or otherwise looped into the panic handler while trying to exit.
Expand Down Expand Up @@ -905,6 +914,22 @@ fn waitForOtherThreadToFinishPanicking() void {
}
}

/// This is to be called by any thread that is attempting to exit the process.
/// If another thread is panicking, this will sleep this thread forever, under
/// the assumption that the crash handler will terminate the program.
///
/// There have been situations in the past where a bundler thread starts
/// panicking, but the main thread ends up marking a test as passing and then
/// exiting with code zero before the crash handler can finish the crash.
pub fn sleepForeverIfAnotherThreadIsCrashing() void {
if (panicking.load(.acquire) > 0) {
// Sleep forever without hammering the CPU
var futex = std.atomic.Value(u32).init(0);
while (true) std.Thread.Futex.wait(&futex, 0);
comptime unreachable;
}
}

/// Each platform is encoded as a single character. It is placed right after the
/// slash after the version, so someone just reading the trace string can tell
/// what platform it came from. L, M, and W are for Linux, macOS, and Windows,
Expand Down Expand Up @@ -1611,7 +1636,7 @@ pub const js_bindings = struct {
const bits = bun.Analytics.packedFeatures();
var buf = std.BoundedArray(u8, 16){};
writeU64AsTwoVLQs(buf.writer(), @bitCast(bits)) catch {
// there is definetly enough space in the bounded array
// there is definitely enough space in the bounded array
unreachable;
};
return bun.String.createLatin1(buf.slice()).toJS(global);
Expand Down
2 changes: 1 addition & 1 deletion src/js_lexer.zig
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub const TypeScriptAccessibilityModifier = tables.TypeScriptAccessibilityModifi
pub const ChildlessJSXTags = tables.ChildlessJSXTags;

fn notimpl() noreturn {
Global.panic("not implemented yet!", .{});
Output.panic("not implemented yet!", .{});
}

pub var emptyJavaScriptString = ([_]u16{0});
Expand Down
10 changes: 5 additions & 5 deletions src/js_parser.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2736,7 +2736,7 @@ pub const StmtsKind = enum {
};

fn notimpl() noreturn {
Global.panic("Not implemented yet!!", .{});
Output.panic("Not implemented yet!!", .{});
}

const ExprBindingTuple = struct {
Expand Down Expand Up @@ -14905,7 +14905,7 @@ fn NewParser_(
p.log.level = .verbose;
p.log.printForLogLevel(panic_stream.writer()) catch unreachable;

Global.panic(fmt ++ "\n{s}", args ++ .{panic_buffer[0..panic_stream.pos]});
Output.panic(fmt ++ "\n{s}", args ++ .{panic_buffer[0..panic_stream.pos]});
}

pub fn parsePrefix(p: *P, level: Level, errors: ?*DeferredErrors, flags: Expr.EFlags) anyerror!Expr {
Expand Down Expand Up @@ -16191,7 +16191,7 @@ fn NewParser_(
}
},
else => {
Global.panic("Unexpected type in export default: {any}", .{s2});
Output.panic("Unexpected type in export default: {any}", .{s2});
},
}
},
Expand Down Expand Up @@ -17486,7 +17486,7 @@ fn NewParser_(
var has_proto = false;
for (e_.properties.slice()) |*property| {
if (property.kind != .spread) {
property.key = p.visitExpr(property.key orelse Global.panic("Expected property key", .{}));
property.key = p.visitExpr(property.key orelse Output.panic("Expected property key", .{}));
const key = property.key.?;
// Forbid duplicate "__proto__" properties according to the specification
if (!property.flags.contains(.is_computed) and
Expand Down Expand Up @@ -20720,7 +20720,7 @@ fn NewParser_(
}
},
else => {
Global.panic("Unexpected binding type in namespace. This is a bug. {any}", .{binding});
Output.panic("Unexpected binding type in namespace. This is a bug. {any}", .{binding});
},
}
}
Expand Down
24 changes: 12 additions & 12 deletions src/js_printer.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2771,7 +2771,7 @@ fn NewPrinter(
if (e.func.name) |sym| {
p.printSpaceBeforeIdentifier();
p.addSourceMapping(sym.loc);
p.printSymbol(sym.ref orelse Global.panic("internal error: expected E.Function's name symbol to have a ref\n{any}", .{e.func}));
p.printSymbol(sym.ref orelse Output.panic("internal error: expected E.Function's name symbol to have a ref\n{any}", .{e.func}));
}

p.printFunc(e.func);
Expand All @@ -2792,7 +2792,7 @@ fn NewPrinter(
if (e.class_name) |name| {
p.print(" ");
p.addSourceMapping(name.loc);
p.printSymbol(name.ref orelse Global.panic("internal error: expected E.Class's name symbol to have a ref\n{any}", .{e}));
p.printSymbol(name.ref orelse Output.panic("internal error: expected E.Class's name symbol to have a ref\n{any}", .{e}));
}
p.printClass(e.*);
if (wrap) {
Expand Down Expand Up @@ -3884,7 +3884,7 @@ fn NewPrinter(
p.print("}");
},
else => {
Global.panic("Unexpected binding of type {any}", .{binding});
Output.panic("Unexpected binding of type {any}", .{binding});
},
}
}
Expand Down Expand Up @@ -3913,8 +3913,8 @@ fn NewPrinter(
.s_function => |s| {
p.printIndent();
p.printSpaceBeforeIdentifier();
const name = s.func.name orelse Global.panic("Internal error: expected func to have a name ref\n{any}", .{s});
const nameRef = name.ref orelse Global.panic("Internal error: expected func to have a name\n{any}", .{s});
const name = s.func.name orelse Output.panic("Internal error: expected func to have a name ref\n{any}", .{s});
const nameRef = name.ref orelse Output.panic("Internal error: expected func to have a name\n{any}", .{s});

if (s.func.flags.contains(.is_export)) {
if (!rewrite_esm_to_cjs) {
Expand Down Expand Up @@ -4038,7 +4038,7 @@ fn NewPrinter(

if (class.class.class_name) |name| {
p.print("class ");
p.printSymbol(name.ref orelse Global.panic("Internal error: Expected class to have a name ref\n{any}", .{class}));
p.printSymbol(name.ref orelse Output.panic("Internal error: Expected class to have a name ref\n{any}", .{class}));
} else {
p.print("class");
}
Expand All @@ -4048,7 +4048,7 @@ fn NewPrinter(
p.printNewline();
},
else => {
Global.panic("Internal error: unexpected export default stmt data {any}", .{s});
Output.panic("Internal error: unexpected export default stmt data {any}", .{s});
},
}
},
Expand Down Expand Up @@ -4444,7 +4444,7 @@ fn NewPrinter(
p.printIndent();
}
p.printSpaceBeforeIdentifier();
p.printSymbol(s.name.ref orelse Global.panic("Internal error: expected label to have a name {any}", .{s}));
p.printSymbol(s.name.ref orelse Output.panic("Internal error: expected label to have a name {any}", .{s}));
p.print(":");
p.printBody(s.stmt);
},
Expand Down Expand Up @@ -4933,9 +4933,9 @@ fn NewPrinter(
const to_print: []const u8 = if (slice.len > 1024) slice[slice.len - 1024 ..] else slice;

if (to_print.len > 0) {
Global.panic("\n<r><red>voluntary crash<r> while printing:<r>\n{s}\n---This is a <b>bug<r>. Not your fault.\n", .{to_print});
Output.panic("\n<r><red>voluntary crash<r> while printing:<r>\n{s}\n---This is a <b>bug<r>. Not your fault.\n", .{to_print});
} else {
Global.panic("\n<r><red>voluntary crash<r> while printing. This is a <b>bug<r>. Not your fault.\n", .{});
Output.panic("\n<r><red>voluntary crash<r> while printing. This is a <b>bug<r>. Not your fault.\n", .{});
}
},
}
Expand Down Expand Up @@ -5162,7 +5162,7 @@ fn NewPrinter(
// for(;)
.s_empty => {},
else => {
Global.panic("Internal error: Unexpected stmt in for loop {any}", .{initSt});
Output.panic("Internal error: Unexpected stmt in for loop {any}", .{initSt});
},
}
}
Expand Down Expand Up @@ -5671,7 +5671,7 @@ pub fn NewWriter(
pub inline fn print(writer: *Self, comptime ValueType: type, str: ValueType) void {
if (FeatureFlags.disable_printing_null) {
if (str == 0) {
Global.panic("Attempted to print null char", .{});
Output.panic("Attempted to print null char", .{});
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/json_parser.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1075,7 +1075,7 @@ fn expectPrintedJSON(_contents: string, expected: string) !void {
const expr = try ParseJSON(&source, &log, default_allocator);

if (log.msgs.items.len > 0) {
Global.panic("--FAIL--\nExpr {s}\nLog: {s}\n--FAIL--", .{ expr, log.msgs.items[0].data.text });
Output.panic("--FAIL--\nExpr {s}\nLog: {s}\n--FAIL--", .{ expr, log.msgs.items[0].data.text });
}

const buffer_writer = try js_printer.BufferWriter.init(default_allocator);
Expand Down
2 changes: 1 addition & 1 deletion src/logger.zig
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ pub const Msg = struct {
}
}

pub fn formatNoWriter(msg: *const Msg, comptime formatterFunc: @TypeOf(Global.panic)) void {
pub fn formatNoWriter(msg: *const Msg, comptime formatterFunc: @TypeOf(Output.panic)) void {
formatterFunc("\n\n{s}: {s}\n{s}\n{s}:{}:{} ({d})", .{
msg.kind.string(),
msg.data.text,
Expand Down
4 changes: 2 additions & 2 deletions src/napi/napi.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1183,10 +1183,10 @@ pub export fn napi_fatal_error(location_ptr: ?[*:0]const u8, location_len: usize

const location = napiSpan(location_ptr, location_len);
if (location.len > 0) {
bun.Global.panic("napi: {s}\n {s}", .{ message, location });
bun.Output.panic("napi: {s}\n {s}", .{ message, location });
}

bun.Global.panic("napi: {s}", .{message});
bun.Output.panic("napi: {s}", .{message});
}
pub export fn napi_create_buffer(env: napi_env, length: usize, data: ?**anyopaque, result: *napi_value) napi_status {
log("napi_create_buffer: {d}", .{length});
Expand Down
2 changes: 1 addition & 1 deletion src/renamer.zig
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub const NoOpRenamer = struct {
if (renamer.symbols.getConst(resolved)) |symbol| {
return symbol.original_name;
} else {
Global.panic("Invalid symbol {s} in {s}", .{ ref, renamer.source.path.text });
Output.panic("Invalid symbol {s} in {s}", .{ ref, renamer.source.path.text });
}
}

Expand Down

0 comments on commit 70ca2b7

Please sign in to comment.