From 238de489c19c1fbbac2ac1fe5109becac4def674 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Tue, 24 Feb 2026 10:42:50 +0800 Subject: [PATCH] Add [basic] reference counting to events Previously, we used a boolean, `_v8_handoff` to detect whether or not an event was handed off to v8. When it _was_ handed off, then we relied on the Global finalizer (or context shutdown) to cleanup the instance. When it wasn't handed off, we could immediately free the instance. The issue is that, under pressure, v8 might finalize the event _before_ we've checked the handoff flag. This was the old code: ```zig const event = try Event.initTrusted(.wrap("DOMContentLoaded"), .{ .bubbles = true }, self); defer if (!event._v8_handoff) event.deinit(false); try self._event_manager.dispatch( self.document.asEventTarget(), event, ); ``` But what happens if, during the call to dispatch, v8 finalizes the event? The defer statement will access event after its been freed. Rather than a boolean, we now track a basic reference count. deinit decreases the reference count, and only frees the object when it reaches 0. Any handoff to v8 automatically increases the reference count by 1. The above code becomes a simpler: ```zig const event = try Event.initTrusted(.wrap("DOMContentLoaded"), .{ .bubbles = true }, self); defer event.deinit(false); try self._event_manager.dispatch( self.document.asEventTarget(), event, ); ``` The deinit is un-conditional. The dispatch function itself increases the RC by 1, and then the v8 handoff increases it to 2. On v8 finalization the RC is decreased to 1. The defer deinit decreases it to 0, at which point it is freed. Fixes WPT /css/css-transitions/properties-value-003.html --- src/browser/EventManager.zig | 6 ++-- src/browser/Factory.zig | 1 + src/browser/js/Local.zig | 10 +++--- src/browser/webapi/Event.zig | 49 +++++++++++++++++++++--------- src/browser/webapi/EventTarget.zig | 3 +- 5 files changed, 47 insertions(+), 22 deletions(-) diff --git a/src/browser/EventManager.zig b/src/browser/EventManager.zig index 53e2c6ad..a1552440 100644 --- a/src/browser/EventManager.zig +++ b/src/browser/EventManager.zig @@ -204,7 +204,8 @@ pub fn dispatch(self: *EventManager, target: *EventTarget, event: *Event) Dispat } pub fn dispatchOpts(self: *EventManager, target: *EventTarget, event: *Event, comptime opts: DispatchOpts) DispatchError!void { - defer if (!event._v8_handoff) event.deinit(false, self.page); + event.acquireRef(); + defer event.deinit(false, self.page); if (comptime IS_DEBUG) { log.debug(.event, "eventManager.dispatch", .{ .type = event._type_string.str(), .bubbles = event._bubbles }); @@ -254,7 +255,8 @@ const DispatchWithFunctionOptions = struct { inject_target: bool = true, }; pub fn dispatchWithFunction(self: *EventManager, target: *EventTarget, event: *Event, function_: ?js.Function, comptime opts: DispatchWithFunctionOptions) !void { - defer if (!event._v8_handoff) event.deinit(false, self.page); + event.acquireRef(); + defer event.deinit(false, self.page); if (comptime IS_DEBUG) { log.debug(.event, "dispatchWithFunction", .{ .type = event._type_string.str(), .context = opts.context, .has_function = function_ != null }); diff --git a/src/browser/Factory.zig b/src/browser/Factory.zig index 44bfcb1e..2e3699f5 100644 --- a/src/browser/Factory.zig +++ b/src/browser/Factory.zig @@ -237,6 +237,7 @@ fn eventInit(arena: Allocator, typ: String, value: anytype) !Event { const time_stamp = (raw_timestamp / 2) * 2; return .{ + ._rc = 0, ._arena = arena, ._type = unionInit(Event.Type, value), ._type_string = typ, diff --git a/src/browser/js/Local.zig b/src/browser/js/Local.zig index 019f22d1..e667be62 100644 --- a/src/browser/js/Local.zig +++ b/src/browser/js/Local.zig @@ -217,7 +217,7 @@ pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object, try ctx.finalizer_callbacks.put(ctx.arena, @intFromPtr(resolved.ptr), fc); } - conditionallyFlagHandoff(value); + conditionallyReference(value); if (@hasDecl(JsApi.Meta, "weak")) { if (comptime IS_DEBUG) { std.debug.assert(JsApi.Meta.weak == true); @@ -1101,14 +1101,14 @@ fn resolveT(comptime T: type, value: *anyopaque) Resolved { }; } -fn conditionallyFlagHandoff(value: anytype) void { +fn conditionallyReference(value: anytype) void { const T = bridge.Struct(@TypeOf(value)); - if (@hasField(T, "_v8_handoff")) { - value._v8_handoff = true; + if (@hasDecl(T, "acquireRef")) { + value.acquireRef(); return; } if (@hasField(T, "_proto")) { - conditionallyFlagHandoff(value._proto); + conditionallyReference(value._proto); } } diff --git a/src/browser/webapi/Event.zig b/src/browser/webapi/Event.zig index 0082682f..2821e9f1 100644 --- a/src/browser/webapi/Event.zig +++ b/src/browser/webapi/Event.zig @@ -25,6 +25,7 @@ const Node = @import("Node.zig"); const String = @import("../../string.zig").String; const Allocator = std.mem.Allocator; +const IS_DEBUG = @import("builtin").mode == .Debug; pub const Event = @This(); @@ -44,13 +45,16 @@ _stop_immediate_propagation: bool = false, _event_phase: EventPhase = .none, _time_stamp: u64, _needs_retargeting: bool = false, -_isTrusted: bool = false, +_is_trusted: bool = false, // There's a period of time between creating an event and handing it off to v8 -// where things can fail. If it does fail, we need to deinit the event. This flag -// when true, tells us the event is registered in the js.Contxt and thus, at -// the very least, will be finalized on context shutdown. -_v8_handoff: bool = false, +// where things can fail. If it does fail, we need to deinit the event. The timing +// window can be difficult to capture, so we use a reference count. +// should be 0, 1, or 2. 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: u8 = 0, pub const EventPhase = enum(u8) { none = 0, @@ -92,7 +96,7 @@ pub fn initTrusted(typ: String, opts_: ?Options, page: *Page) !*Event { return initWithTrusted(arena, typ, opts_, true); } -fn initWithTrusted(arena: Allocator, typ: String, opts_: ?Options, trusted: bool) !*Event { +fn initWithTrusted(arena: Allocator, typ: String, opts_: ?Options, comptime trusted: bool) !*Event { const opts = opts_ orelse Options{}; // Round to 2ms for privacy (browsers do this) @@ -108,7 +112,7 @@ fn initWithTrusted(arena: Allocator, typ: String, opts_: ?Options, trusted: bool ._cancelable = opts.cancelable, ._composed = opts.composed, ._type_string = typ, - ._isTrusted = trusted, + ._is_trusted = trusted, }; return event; } @@ -131,9 +135,26 @@ pub fn initEvent( self._prevent_default = false; } +pub fn acquireRef(self: *Event) void { + self._rc += 1; +} + pub fn deinit(self: *Event, shutdown: bool, page: *Page) void { - _ = shutdown; - page.releaseArena(self._arena); + if (shutdown) { + page.releaseArena(self._arena); + return; + } + + const rc = self._rc; + if (comptime IS_DEBUG) { + std.debug.assert(rc != 0); + } + + if (rc == 1) { + page.releaseArena(self._arena); + } else { + self._rc = rc - 1; + } } pub fn as(self: *Event, comptime T: type) *T { @@ -235,15 +256,15 @@ pub fn getTimeStamp(self: *const Event) u64 { } pub fn setTrusted(self: *Event) void { - self._isTrusted = true; + self._is_trusted = true; } pub fn setUntrusted(self: *Event) void { - self._isTrusted = false; + self._is_trusted = false; } pub fn getIsTrusted(self: *const Event) bool { - return self._isTrusted; + return self._is_trusted; } pub fn composedPath(self: *Event, page: *Page) ![]const *EventTarget { @@ -401,8 +422,8 @@ pub fn populatePrototypes(self: anytype, opts: anytype, trusted: bool) void { } // Set isTrusted at the Event level (base of prototype chain) - if (T == Event or @hasField(T, "_isTrusted")) { - self._isTrusted = trusted; + if (T == Event or @hasField(T, "is_trusted")) { + self._is_trusted = trusted; } } diff --git a/src/browser/webapi/EventTarget.zig b/src/browser/webapi/EventTarget.zig index 28b8ee93..d322b68d 100644 --- a/src/browser/webapi/EventTarget.zig +++ b/src/browser/webapi/EventTarget.zig @@ -55,7 +55,8 @@ pub fn dispatchEvent(self: *EventTarget, event: *Event, page: *Page) !bool { if (event._event_phase != .none) { return error.InvalidStateError; } - event._isTrusted = false; + event._is_trusted = false; + event.acquireRef(); try page._event_manager.dispatch(self, event); return !event._cancelable or !event._prevent_default; }