Merge pull request #3011 from lightpanda-io/rc-overflow

overflow: Make RC a u32

src/browser/tests/net/url_search_params.html

@@ -19,6 +19,17 @@
   }
 </script>
 
+<script id=rcStress>
+  // regression for when RC was generic, held a u8 and would overflow
+  const stress = new URLSearchParams('a=1&b=2');
+  const iters = [];
+  for (let i = 0; i < 300; i++) {
+    iters.push(stress.keys());
+  }
+  // as long as this test doesn't cause a crash, we're ok
+  testing.expectTrue(true);
+</script>
+
 <script id=urlSearchParams>
   const inputs = [
     [["over", "9000!!"], ["abc", "123"], ["key1", ""], ["key2", ""]],

src/browser/webapi/AbstractRange.zig

@@ -32,7 +32,7 @@ const AbstractRange = @This();
 
 pub const _prototype_root = true;
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _type: Type,
 _arena: Allocator,
 _end_offset: u32,

src/browser/webapi/Blob.zig

@@ -35,7 +35,7 @@ const Blob = @This();
 pub const _prototype_root = true;
 
 _type: Type,
-_rc: lp.RC(u32),
+_rc: lp.RC,
 
 _arena: Allocator,
 

src/browser/webapi/CryptoKey.zig

@@ -30,7 +30,7 @@ const Allocator = std.mem.Allocator;
 /// generateKey(), deriveKey(), importKey(), or unwrapKey().
 const CryptoKey = @This();
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _arena: Allocator = undefined,
 /// Algorithm being used.
 _type: Type,

src/browser/webapi/DOMMatrixReadOnly.zig

@@ -30,7 +30,7 @@ const DOMMatrixReadOnly = @This();
 pub const _prototype_root = true;
 
 _type: Type,
-_rc: lp.RC(u8),
+_rc: lp.RC,
 _arena: Allocator,
 
 // Stored column-major, matching the spec's mAB naming where A is the column

src/browser/webapi/DOMNodeIterator.zig

@@ -29,7 +29,7 @@ pub const FilterOpts = NodeFilter.FilterOpts;
 
 const DOMNodeIterator = @This();
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _root: *Node,
 _what_to_show: u32,
 _filter: NodeFilter,

src/browser/webapi/DOMPointReadOnly.zig

@@ -31,7 +31,7 @@ const DOMPointReadOnly = @This();
 pub const _prototype_root = true;
 
 _type: Type,
-_rc: lp.RC(u8),
+_rc: lp.RC,
 _arena: Allocator,
 
 _x: f64,

src/browser/webapi/DOMTreeWalker.zig

@@ -28,7 +28,7 @@ pub const FilterOpts = NodeFilter.FilterOpts;
 
 const DOMTreeWalker = @This();
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _root: *Node,
 _what_to_show: u32,
 _filter: NodeFilter,

src/browser/webapi/DataTransfer.zig

@@ -50,7 +50,7 @@ pub fn registerTypes() []const type {
 _arena: Allocator,
 // Refcounted so the GC weak-finalizer (or page teardown) releases the pooled
 // arena exactly once; mirrors Blob's lifecycle.
-_rc: lp.RC(u32) = .{},
+_rc: lp.RC = .{},
 _items: std.ArrayList(*DataTransferItem) = .{},
 _item_list: *DataTransferItemList,
 // FileList lives on the factory slab and is frame-tracked, so each File ref it

src/browser/webapi/Event.zig

@@ -65,7 +65,7 @@ _time_origin: u64 = 0,
 // - 0: no reference, always a transient state going to either 1 or about to be deinit'd
 // - 1: either zig or v8 have a reference
 // - 2: both zig and v8 have a reference
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 
 pub const EventPhase = enum(u8) {
     none = 0,

src/browser/webapi/FileReader.zig

@@ -33,7 +33,7 @@ const Allocator = std.mem.Allocator;
 /// https://developer.mozilla.org/en-US/docs/Web/API/FileReader
 const FileReader = @This();
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _exec: *Execution,
 _proto: *EventTarget,
 _arena: Allocator,

src/browser/webapi/ImageData.zig

@@ -28,7 +28,7 @@ const Execution = js.Execution;
 /// https://developer.mozilla.org/en-US/docs/Web/API/ImageData/ImageData
 const ImageData = @This();
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _width: u32,
 _height: u32,
 _data: js.ArrayBufferRef(.uint8_clamped).Global,

src/browser/webapi/IntersectionObserver.zig

@@ -39,7 +39,7 @@ pub fn registerTypes() []const type {
 
 const IntersectionObserver = @This();
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _arena: Allocator,
 _callback: js.Function.Global,
 _observing: std.ArrayList(*Element) = .{},
@@ -316,7 +316,7 @@ pub fn deliverEntries(self: *IntersectionObserver, frame: *Frame) !void {
 }
 
 pub const IntersectionObserverEntry = struct {
-    _rc: lp.RC(u8) = .{},
+    _rc: lp.RC = .{},
     _arena: Allocator,
     _time: f64,
     _target: *Element,

src/browser/webapi/Location.zig

@@ -28,7 +28,7 @@ const Frame = @import("../Frame.zig");
 const Location = @This();
 
 _url: *URL,
-_rc: lp.RC(u32) = .{},
+_rc: lp.RC = .{},
 
 pub fn init(raw_url: []const u8, frame: *Frame) !*Location {
     const url = try URL.init(raw_url, null, &frame.js.execution);

src/browser/webapi/MutationObserver.zig

@@ -39,7 +39,7 @@ pub fn registerTypes() []const type {
 
 const MutationObserver = @This();
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _arena: Allocator,
 _callback: js.Function.Global,
 _observing: std.ArrayList(Observing) = .{},
@@ -357,7 +357,7 @@ pub fn deliverRecords(self: *MutationObserver, frame: *Frame) !void {
 }
 
 pub const MutationRecord = struct {
-    _rc: lp.RC(u8) = .{},
+    _rc: lp.RC = .{},
     _type: Type,
     _target: *Node,
     _arena: Allocator,

src/browser/webapi/Notification.zig

@@ -29,7 +29,7 @@ const Allocator = std.mem.Allocator;
 
 const Notification = @This();
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _arena: Allocator,
 _proto: *EventTarget,
 _title: []const u8,

src/browser/webapi/Permissions.zig

@@ -61,7 +61,7 @@ pub fn query(_: *const Permissions, qd: QueryDescriptor, exec: *const Execution)
 }
 
 const PermissionStatus = struct {
-    _rc: lp.RC(u8) = .{},
+    _rc: lp.RC = .{},
     _arena: Allocator,
     _name: []const u8,
     _state: State,

src/browser/webapi/Selection.zig

@@ -33,7 +33,7 @@ const Selection = @This();
 
 pub const SelectionDirection = enum { backward, forward, none };
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _range: ?*Range = null,
 _direction: SelectionDirection = .none,
 

src/browser/webapi/URL.zig

@@ -33,7 +33,7 @@ _url: *U.Url = undefined,
 _port: [5]u8 = undefined,
 _search_params: ?*URLSearchParams = null,
 /// We have to track lifetime of URL to free `_url`.
-_rc: lp.RC(u32) = .{},
+_rc: lp.RC = .{},
 
 // convenience
 pub const resolve = @import("../URL.zig").resolve;

src/browser/webapi/XPathExpression.zig

@@ -42,7 +42,7 @@ const Allocator = std.mem.Allocator;
 
 const XPathExpression = @This();
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _arena: Allocator,
 _expr: *const xpath.Ast.Expr,
 

src/browser/webapi/XPathResult.zig

@@ -75,7 +75,7 @@ const Value = union(enum) {
     nodes: []const *Node,
 };
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _arena: Allocator,
 _type: u16,
 _value: Value,

src/browser/webapi/animation/Animation.zig

@@ -34,7 +34,7 @@ const PlayState = enum {
     finished,
 };
 
-_rc: lp.RC(u32) = .{},
+_rc: lp.RC = .{},
 _frame: *Frame,
 _arena: Allocator,
 

src/browser/webapi/collections/DOMStringList.zig

@@ -43,7 +43,7 @@ pub fn registerTypes() []const type {
 
 pub const DOMStringList = @This();
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _arena: Allocator,
 _items: []const []const u8,
 

src/browser/webapi/collections/HTMLCollection.zig

@@ -60,7 +60,7 @@ _data: union(Mode) {
     form: NodeLive(.form),
     empty: void,
 },
-_rc: lp.RC(u32) = .{},
+_rc: lp.RC = .{},
 
 pub fn deinit(self: *HTMLCollection, page: *Page) void {
     page.factory.destroy(self);

src/browser/webapi/collections/NodeList.zig

@@ -39,7 +39,7 @@ _data: union(enum) {
     radio_node_list: *RadioNodeList,
     name: NodeLive(.name),
 },
-_rc: lp.RC(u32) = .{},
+_rc: lp.RC = .{},
 
 pub fn deinit(self: *NodeList, page: *Page) void {
     switch (self._data) {

src/browser/webapi/collections/iterator.zig

@@ -27,7 +27,7 @@ pub fn Entry(comptime Inner: type, comptime field: ?[]const u8) type {
 
     return struct {
         _inner: Inner,
-        _rc: lp.RC(u8) = .{},
+        _rc: lp.RC = .{},
 
         const Self = @This();
 

src/browser/webapi/css/FontFace.zig

@@ -27,7 +27,7 @@ const Allocator = std.mem.Allocator;
 
 const FontFace = @This();
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _arena: Allocator,
 _family: []const u8,
 

src/browser/webapi/css/FontFaceSet.zig

@@ -32,7 +32,7 @@ const Allocator = std.mem.Allocator;
 
 const FontFaceSet = @This();
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _proto: *EventTarget,
 _arena: Allocator,
 

src/browser/webapi/encoding/TextDecoder.zig

@@ -28,7 +28,7 @@ const Allocator = std.mem.Allocator;
 
 const TextDecoder = @This();
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _fatal: bool,
 _arena: Allocator,
 _ignore_bom: bool,

src/browser/webapi/net/EventSource.zig

@@ -39,7 +39,7 @@ const IS_DEBUG = @import("builtin").mode == .Debug;
 // https://html.spec.whatwg.org/multipage/server-sent-events.html
 const EventSource = @This();
 
-_rc: lp.RC(u16) = .{},
+_rc: lp.RC = .{},
 _exec: *const Execution,
 _proto: *EventTarget,
 _arena: Allocator,

src/browser/webapi/net/FormData.zig

@@ -34,7 +34,7 @@ const Allocator = std.mem.Allocator;
 
 const FormData = @This();
 
-_rc: lp.RC(u8),
+_rc: lp.RC,
 
 _arena: Allocator,
 _entries: std.ArrayList(Entry),

src/browser/webapi/net/Request.zig

@@ -36,7 +36,7 @@ const Allocator = std.mem.Allocator;
 
 const Request = @This();
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _url: [:0]const u8,
 _method: http.Method,
 _headers: ?*Headers,

src/browser/webapi/net/Response.zig

@@ -44,7 +44,7 @@ pub const Type = enum {
     opaqueredirect,
 };
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _status: u16,
 _arena: Allocator,
 _headers: *Headers,

src/browser/webapi/net/URLSearchParams.zig

@@ -41,7 +41,7 @@ pub fn registerTypes() []const type {
 
 const URLSearchParams = @This();
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _arena: Allocator,
 _params: KeyValueList,
 

src/browser/webapi/net/WebSocket.zig

@@ -40,7 +40,7 @@ const IS_DEBUG = @import("builtin").mode == .Debug;
 
 const WebSocket = @This();
 
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _exec: *const Execution,
 _proto: *EventTarget,
 _arena: Allocator,

src/browser/webapi/net/XMLHttpRequest.zig

@@ -42,7 +42,7 @@ const Allocator = std.mem.Allocator;
 const IS_DEBUG = @import("builtin").mode == .Debug;
 
 const XMLHttpRequest = @This();
-_rc: lp.RC(u8) = .{},
+_rc: lp.RC = .{},
 _exec: *const Execution,
 _proto: *XMLHttpRequestEventTarget,
 _upload: ?*XMLHttpRequestUpload = null,

src/browser/webapi/storage/idb/IDBTransaction.zig

@@ -59,7 +59,7 @@ _durability: Durability = .default,
 // owned object, one while a drain task is scheduled, one while parked on the
 // engine's connection gate (gate ownership needs no ref of its own: it's only
 // ever held while a drain task exists).
-_rc: lp.RC(u32) = .{},
+_rc: lp.RC = .{},
 _arena: Allocator,
 
 // v8 handles owned by the transaction, swept (reset) in deinit. Slots are

src/lightpanda.zig

@@ -329,42 +329,42 @@ noinline fn assertionFailure(comptime ctx: []const u8, args: anytype) noreturn {
 const rc_canary: u32 = 0x52434E54;
 const rc_poison: u32 = 0xDEADC0DE;
 
-// Reference counting helper
-pub fn RC(comptime T: type) type {
-    return struct {
-        _refs: std.atomic.Value(T) = .init(0),
-        _canary: u32 = rc_canary,
+// Reference counting helper. The count is a u32: a u8 silently wrapped at 256
+// concurrent refs (e.g. hundreds of live iterators on one URLSearchParams),
+// causing a premature deinit and a poisoned "release overflow" crash.
+pub const RC = struct {
+    _refs: std.atomic.Value(u32) = .init(0),
+    _canary: u32 = rc_canary,
 
-        pub fn init(refs: T) @This() {
-            return .{ ._refs = .init(refs) };
-        }
+    pub fn init(refs: u32) RC {
+        return .{ ._refs = .init(refs) };
+    }
 
-        pub fn acquire(self: *@This()) void {
-            _ = self._refs.fetchAdd(1, .monotonic);
-        }
+    pub fn acquire(self: *RC) void {
+        _ = self._refs.fetchAdd(1, .monotonic);
+    }
 
-        pub fn release(self: *@This(), value: anytype, page: *Page) void {
-            const prev = self._refs.fetchSub(1, .acq_rel);
-            assert(prev > 0, "release overflow", .{
-                .type = @typeName(@TypeOf(value)),
-                .canary = self._canary, // rc_canary=live/accounting, rc_poison=double-release, else=reuse
-                .refs = prev,
-                .ptr = @intFromPtr(value),
-            });
-            if (prev == 1) {
-                // Mark dead before deinit frees this memory, so a stale
-                // weak-callback re-fire reads rc_poison instead of a
-                // misleadingly-intact canary.
-                self._canary = rc_poison;
-                value.deinit(page);
-            }
+    pub fn release(self: *RC, value: anytype, page: *Page) void {
+        const prev = self._refs.fetchSub(1, .acq_rel);
+        assert(prev > 0, "release overflow", .{
+            .type = @typeName(@TypeOf(value)),
+            .canary = self._canary, // rc_canary=live/accounting, rc_poison=double-release, else=reuse
+            .refs = prev,
+            .ptr = @intFromPtr(value),
+        });
+        if (prev == 1) {
+            // Mark dead before deinit frees this memory, so a stale
+            // weak-callback re-fire reads rc_poison instead of a
+            // misleadingly-intact canary.
+            self._canary = rc_poison;
+            value.deinit(page);
         }
+    }
 
-        pub fn format(self: @This(), writer: *std.Io.Writer) !void {
-            return writer.print("{d}", .{self._refs.load(.monotonic)});
-        }
-    };
-}
+    pub fn format(self: RC, writer: *std.Io.Writer) !void {
+        return writer.print("{d}", .{self._refs.load(.monotonic)});
+    }
+};
 
 const testing = @import("testing.zig");
 test "writeJsonEnvelope: null frame" {