Merge pull request #1359 from lightpanda-io/crash_handler

Improve crash handling

Author: karlseguin

build.zig

@@ -48,6 +48,7 @@ pub fn build(b: *Build) !void {
             .sanitize_c = enable_csan,
             .sanitize_thread = enable_tsan,
         });
+        mod.addImport("lightpanda", mod); // allow circular "lightpanda" import
 
         try addDependencies(b, mod, opts, prebuilt_v8_path);
 

src/Notification.zig

@@ -17,6 +17,7 @@
 // along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 const std = @import("std");
+const lp = @import("lightpanda");
 
 const log = @import("log.zig");
 const Page = @import("browser/Page.zig");
@@ -241,7 +242,7 @@ pub fn unregister(self: *Notification, comptime event: EventType, receiver: anyt
     if (listeners.items.len == 0) {
         listeners.deinit(self.allocator);
         const removed = self.listeners.remove(@intFromPtr(receiver));
-        std.debug.assert(removed == true);
+        lp.assert(removed == true, "Notification.unregister", .{ .type = event });
     }
 }
 

src/Server.zig

@@ -17,6 +17,7 @@
 // along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 const std = @import("std");
+const lp = @import("lightpanda");
 const builtin = @import("builtin");
 
 const net = std.net;
@@ -157,7 +158,7 @@ fn readLoop(self: *Server, socket: posix.socket_t, timeout_ms: u32) !void {
     });
     defer http.removeCDPClient();
 
-    std.debug.assert(client.mode == .http);
+    lp.assert(client.mode == .http, "Server.readLoop invalid mode", .{});
     while (true) {
         if (http.poll(timeout_ms) != .cdp_socket) {
             log.info(.app, "CDP timeout", .{});
@@ -236,7 +237,7 @@ pub const Client = struct {
         const socket_flags = try posix.fcntl(socket, posix.F.GETFL, 0);
         const nonblocking = @as(u32, @bitCast(posix.O{ .NONBLOCK = true }));
         // we expect the socket to come to us as nonblocking
-        std.debug.assert(socket_flags & nonblocking == nonblocking);
+        lp.assert(socket_flags & nonblocking == nonblocking, "Client.init blocking", .{});
 
         var reader = try Reader(true).init(server.allocator);
         errdefer reader.deinit();
@@ -311,7 +312,7 @@ pub const Client = struct {
     }
 
     fn processHTTPRequest(self: *Client) !bool {
-        std.debug.assert(self.reader.pos == 0);
+        lp.assert(self.reader.pos == 0, "Client.HTTP pos", .{ .pos = self.reader.pos });
         const request = self.reader.buf[0..self.reader.len];
 
         if (request.len > MAX_HTTP_REQUEST_SIZE) {
@@ -592,8 +593,7 @@ pub const Client = struct {
                     // blocking and switch it back to non-blocking after the write
                     // is complete. Doesn't seem particularly efficiently, but
                     // this should virtually never happen.
-                    std.debug.assert(changed_to_blocking == false);
-                    log.debug(.app, "CDP write would block", .{});
+                    lp.assert(changed_to_blocking == false, "Client.double block", .{});
                     changed_to_blocking = true;
                     _ = try posix.fcntl(self.socket, posix.F.SETFL, self.socket_flags & ~@as(u32, @bitCast(posix.O{ .NONBLOCK = true })));
                     continue :LOOP;
@@ -821,7 +821,7 @@ fn Reader(comptime EXPECT_MASK: bool) type {
             const pos = self.pos;
             const len = self.len;
 
-            std.debug.assert(pos <= len);
+            lp.assert(pos <= len, "Client.Reader.compact precondition", .{ .pos = pos, .len = len });
 
             // how many (if any) partial bytes do we have
             const partial_bytes = len - pos;
@@ -842,7 +842,7 @@ fn Reader(comptime EXPECT_MASK: bool) type {
                 const next_message_len = length_meta.@"1";
                 // if this isn't true, then we have a full message and it
                 // should have been processed.
-                std.debug.assert(next_message_len > partial_bytes);
+                lp.assert(pos <= len, "Client.Reader.compact postcondition", .{ .next_len = next_message_len, .partial = partial_bytes });
 
                 const missing_bytes = next_message_len - partial_bytes;
 
@@ -929,7 +929,7 @@ fn fillWebsocketHeader(buf: std.ArrayListUnmanaged(u8)) []const u8 {
 // makes the assumption that our caller reserved the first
 // 10 bytes for the header
 fn websocketHeader(buf: []u8, op_code: OpCode, payload_len: usize) []const u8 {
-    std.debug.assert(buf.len == 10);
+    lp.assert(buf.len == 10, "Websocket.Header", .{ .len = buf.len });
 
     const len = payload_len;
     buf[0] = 128 | @intFromEnum(op_code); // fin | opcode

src/browser/Factory.zig

@@ -17,10 +17,8 @@
 // along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 const std = @import("std");
-const assert = std.debug.assert;
 const builtin = @import("builtin");
 const reflect = @import("reflect.zig");
-const IS_DEBUG = builtin.mode == .Debug;
 
 const log = @import("../log.zig");
 const String = @import("../string.zig").String;
@@ -38,6 +36,9 @@ const XMLHttpRequestEventTarget = @import("webapi/net/XMLHttpRequestEventTarget.
 const Blob = @import("webapi/Blob.zig");
 const AbstractRange = @import("webapi/AbstractRange.zig");
 
+const IS_DEBUG = builtin.mode == .Debug;
+const assert = std.debug.assert;
+
 const Factory = @This();
 _page: *Page,
 _slab: SlabAllocator,

src/browser/Page.zig

@@ -18,6 +18,7 @@
 
 const std = @import("std");
 const JS = @import("js/js.zig");
+const lp = @import("lightpanda");
 const builtin = @import("builtin");
 
 const Allocator = std.mem.Allocator;
@@ -786,7 +787,9 @@ fn _wait(self: *Page, wait_ms: u32) !Session.WaitResult {
                     // an extra socket, so it should not be possibl to
                     // get an cdp_socket message when exit_when_done
                     // is true.
-                    std.debug.assert(exit_when_done == false);
+                    if (IS_DEBUG) {
+                        std.debug.assert(exit_when_done == false);
+                    }
 
                     // data on a socket we aren't handling, return to caller
                     return .cdp_socket;
@@ -822,7 +825,9 @@ fn _wait(self: *Page, wait_ms: u32) !Session.WaitResult {
                     // we don't need to consider http_client.intercepted here
                     // because exit_when_done is true, and that can only be
                     // the case when interception isn't possible.
-                    std.debug.assert(http_client.intercepted == 0);
+                    if (comptime IS_DEBUG) {
+                        std.debug.assert(http_client.intercepted == 0);
+                    }
 
                     const ms = ms_to_next_task orelse blk: {
                         if (wait_ms - ms_remaining < 100) {
@@ -1019,7 +1024,9 @@ fn getElementIdMap(page: *Page, node: *Node) ElementIdMaps {
                 };
             }
             // Detached nodes should not have IDs registered
-            std.debug.assert(false);
+            if (IS_DEBUG) {
+                std.debug.assert(false);
+            }
             return .{
                 .lookup = &page.document._elements_by_id,
                 .removed_ids = &page.document._removed_ids,
@@ -1265,14 +1272,14 @@ pub fn deliverSlotchangeEvents(self: *Page) void {
 }
 
 fn notifyNetworkIdle(self: *Page) void {
-    std.debug.assert(self._notified_network_idle == .done);
+    lp.assert(self._notified_network_idle == .done, "Page.notifyNetworkIdle", .{});
     self._session.browser.notification.dispatch(.page_network_idle, &.{
         .timestamp = timestamp(.monotonic),
     });
 }
 
 fn notifyNetworkAlmostIdle(self: *Page) void {
-    std.debug.assert(self._notified_network_almost_idle == .done);
+    lp.assert(self._notified_network_almost_idle == .done, "Page.notifyNetworkAlmostIdle", .{});
     self._session.browser.notification.dispatch(.page_network_almost_idle, &.{
         .timestamp = timestamp(.monotonic),
     });
@@ -1298,7 +1305,7 @@ pub fn appendNew(self: *Page, parent: *Node, child: Node.NodeOrText) !void {
         },
     };
 
-    std.debug.assert(node._parent == null);
+    lp.assert(node._parent == null, "Page.appendNew", .{});
     try self._insertNodeRelative(true, parent, node, .append, .{
         // this opts has no meaning since we're passing `true` as the first
         // parameter, which indicates this comes from the parser, and has its
@@ -2213,7 +2220,7 @@ pub fn removeNode(self: *Page, parent: *Node, child: *Node, opts: RemoveNodeOpts
     const children = parent._children.?;
     switch (children.*) {
         .one => |n| {
-            std.debug.assert(n == child);
+            lp.assert(n == child, "Page.removeNode.one", .{});
             parent._children = null;
             self._factory.destroy(children);
         },
@@ -2340,7 +2347,8 @@ pub fn insertNodeRelative(self: *Page, parent: *Node, child: *Node, relative: In
 }
 pub fn _insertNodeRelative(self: *Page, comptime from_parser: bool, parent: *Node, child: *Node, relative: InsertNodeRelative, opts: InsertNodeOpts) !void {
     // caller should have made sure this was the case
-    std.debug.assert(child._parent == null);
+
+    lp.assert(child._parent == null, "Page.insertNodeRelative parent", .{ .url = self.url });
 
     const children = blk: {
         // expand parent._children so that it can take another child
@@ -2369,14 +2377,14 @@ pub fn _insertNodeRelative(self: *Page, comptime from_parser: bool, parent: *Nod
         },
         .after => |ref_node| {
             // caller should have made sure this was the case
-            std.debug.assert(ref_node._parent.? == parent);
+            lp.assert(ref_node._parent.? == parent, "Page.insertNodeRelative after", .{ .url = self.url });
             // if ref_node is in parent, and expanded _children above to
             // accommodate another child, then `children` must be a list
             children.list.insertAfter(&ref_node._child_link, &child._child_link);
         },
         .before => |ref_node| {
             // caller should have made sure this was the case
-            std.debug.assert(ref_node._parent.? == parent);
+            lp.assert(ref_node._parent.? == parent, "Page.insertNodeRelative before", .{ .url = self.url });
             // if ref_node is in parent, and expanded _children above to
             // accommodate another child, then `children` must be a list
             children.list.insertBefore(&ref_node._child_link, &child._child_link);
@@ -2654,7 +2662,7 @@ pub fn parseHtmlAsChildren(self: *Page, node: *Node, html: []const u8) !void {
     // https://github.com/servo/html5ever/issues/583
     const children = node._children orelse return;
     const first = children.one;
-    std.debug.assert(first.is(Element.Html.Html) != null);
+    lp.assert(first.is(Element.Html.Html) != null, "Page.parseHtmlAsChildren root", .{ .type = first._type });
     node._children = first._children;
 
     if (self.hasMutationObservers()) {

src/browser/Scheduler.zig

@@ -95,7 +95,9 @@ fn runQueue(self: *Scheduler, queue: *Queue) !?u64 {
 
         if (repeat_in_ms) |ms| {
             // Task cannot be repeated immediately, and they should know that
-            std.debug.assert(ms != 0);
+            if (comptime IS_DEBUG) {
+                std.debug.assert(ms != 0);
+            }
             task.run_at = now + ms;
             try self.low_priority.add(task);
         }

src/browser/ScriptManager.zig

@@ -17,6 +17,7 @@
 // along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 const std = @import("std");
+const lp = @import("lightpanda");
 const builtin = @import("builtin");
 
 const js = @import("js/js.zig");
@@ -484,7 +485,7 @@ pub fn getAsyncImport(self: *ScriptManager, url: [:0]const u8, cb: ImportAsync.C
 // Called from the Page to let us know it's done parsing the HTML. Necessary that
 // we know this so that we know that we can start evaluating deferred scripts.
 pub fn staticScriptsDone(self: *ScriptManager) void {
-    std.debug.assert(self.static_scripts_done == false);
+    lp.assert(self.static_scripts_done == false, "ScriptManager.staticScriptsDone", .{});
     self.static_scripts_done = true;
     self.evaluate();
 }
@@ -675,7 +676,7 @@ pub const Script = struct {
         // set `CURLOPT_SUPPRESS_CONNECT_HEADERS` and CONNECT to a proxy, this
         // will fail. This assertion exists to catch incorrect assumptions about
         // how libcurl works, or about how we've configured it.
-        std.debug.assert(self.source.remote.capacity == 0);
+        lp.assert(self.source.remote.capacity == 0, "ScriptManager.HeaderCallback", .{ .capacity = self.source.remote.capacity });
         var buffer = self.manager.buffer_pool.get();
         if (transfer.getContentLength()) |cl| {
             try buffer.ensureTotalCapacity(self.manager.allocator, cl);
@@ -750,10 +751,12 @@ pub const Script = struct {
 
     fn eval(self: *Script, page: *Page) void {
         // never evaluated, source is passed back to v8, via callbacks.
-        std.debug.assert(self.mode != .import_async);
+        if (comptime IS_DEBUG) {
+            std.debug.assert(self.mode != .import_async);
 
-        // never evaluated, source is passed back to v8 when asked for it.
-        std.debug.assert(self.mode != .import);
+            // never evaluated, source is passed back to v8 when asked for it.
+            std.debug.assert(self.mode != .import);
+        }
 
         if (page.isGoingAway()) {
             // don't evaluate scripts for a dying page.

src/browser/Session.zig

@@ -17,6 +17,7 @@
 // along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 const std = @import("std");
+const lp = @import("lightpanda");
 
 const log = @import("../log.zig");
 
@@ -92,7 +93,7 @@ pub fn deinit(self: *Session) void {
 // NOTE: the caller is not the owner of the returned value,
 // the pointer on Page is just returned as a convenience
 pub fn createPage(self: *Session) !*Page {
-    std.debug.assert(self.page == null);
+    lp.assert(self.page == null, "Session.createPage - page not null", .{});
 
     const page_arena = &self.browser.page_arena;
     _ = page_arena.reset(.{ .retain_with_limit = 1 * 1024 * 1024 });
@@ -116,8 +117,7 @@ pub fn createPage(self: *Session) !*Page {
 pub fn removePage(self: *Session) void {
     // Inform CDP the page is going to be removed, allowing other worlds to remove themselves before the main one
     self.browser.notification.dispatch(.page_remove, .{});
-
-    std.debug.assert(self.page != null);
+    lp.assert(self.page != null, "Session.removePage - page is null", .{});
 
     self.page.?.deinit();
     self.page = null;

src/browser/URL.zig

@@ -17,6 +17,7 @@
 // along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 const std = @import("std");
+const lp = @import("lightpanda");
 const Allocator = std.mem.Allocator;
 
 const ResolveOpts = struct {
@@ -93,7 +94,7 @@ pub fn resolve(allocator: Allocator, base: [:0]const u8, path: anytype, comptime
         }
 
         if (std.mem.startsWith(u8, out[in_i..], "../")) {
-            std.debug.assert(out[out_i - 1] == '/');
+            lp.assert(out[out_i - 1] == '/', "URL.resolve", .{ .out = out });
 
             if (out_i > path_marker) {
                 // go back before the /