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
This commit is contained in:
Karl Seguin
2026-02-24 10:42:50 +08:00
parent 6b4db330d8
commit 238de489c1
5 changed files with 47 additions and 22 deletions

View File

@@ -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 { 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) { if (comptime IS_DEBUG) {
log.debug(.event, "eventManager.dispatch", .{ .type = event._type_string.str(), .bubbles = event._bubbles }); log.debug(.event, "eventManager.dispatch", .{ .type = event._type_string.str(), .bubbles = event._bubbles });
@@ -254,7 +255,8 @@ const DispatchWithFunctionOptions = struct {
inject_target: bool = true, inject_target: bool = true,
}; };
pub fn dispatchWithFunction(self: *EventManager, target: *EventTarget, event: *Event, function_: ?js.Function, comptime opts: DispatchWithFunctionOptions) !void { 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) { if (comptime IS_DEBUG) {
log.debug(.event, "dispatchWithFunction", .{ .type = event._type_string.str(), .context = opts.context, .has_function = function_ != null }); log.debug(.event, "dispatchWithFunction", .{ .type = event._type_string.str(), .context = opts.context, .has_function = function_ != null });

View File

@@ -237,6 +237,7 @@ fn eventInit(arena: Allocator, typ: String, value: anytype) !Event {
const time_stamp = (raw_timestamp / 2) * 2; const time_stamp = (raw_timestamp / 2) * 2;
return .{ return .{
._rc = 0,
._arena = arena, ._arena = arena,
._type = unionInit(Event.Type, value), ._type = unionInit(Event.Type, value),
._type_string = typ, ._type_string = typ,

View File

@@ -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); try ctx.finalizer_callbacks.put(ctx.arena, @intFromPtr(resolved.ptr), fc);
} }
conditionallyFlagHandoff(value); conditionallyReference(value);
if (@hasDecl(JsApi.Meta, "weak")) { if (@hasDecl(JsApi.Meta, "weak")) {
if (comptime IS_DEBUG) { if (comptime IS_DEBUG) {
std.debug.assert(JsApi.Meta.weak == true); 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)); const T = bridge.Struct(@TypeOf(value));
if (@hasField(T, "_v8_handoff")) { if (@hasDecl(T, "acquireRef")) {
value._v8_handoff = true; value.acquireRef();
return; return;
} }
if (@hasField(T, "_proto")) { if (@hasField(T, "_proto")) {
conditionallyFlagHandoff(value._proto); conditionallyReference(value._proto);
} }
} }

View File

@@ -25,6 +25,7 @@ const Node = @import("Node.zig");
const String = @import("../../string.zig").String; const String = @import("../../string.zig").String;
const Allocator = std.mem.Allocator; const Allocator = std.mem.Allocator;
const IS_DEBUG = @import("builtin").mode == .Debug;
pub const Event = @This(); pub const Event = @This();
@@ -44,13 +45,16 @@ _stop_immediate_propagation: bool = false,
_event_phase: EventPhase = .none, _event_phase: EventPhase = .none,
_time_stamp: u64, _time_stamp: u64,
_needs_retargeting: bool = false, _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 // 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 // where things can fail. If it does fail, we need to deinit the event. The timing
// when true, tells us the event is registered in the js.Contxt and thus, at // window can be difficult to capture, so we use a reference count.
// the very least, will be finalized on context shutdown. // should be 0, 1, or 2. 0
_v8_handoff: bool = false, // - 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) { pub const EventPhase = enum(u8) {
none = 0, none = 0,
@@ -92,7 +96,7 @@ pub fn initTrusted(typ: String, opts_: ?Options, page: *Page) !*Event {
return initWithTrusted(arena, typ, opts_, true); 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{}; const opts = opts_ orelse Options{};
// Round to 2ms for privacy (browsers do this) // 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, ._cancelable = opts.cancelable,
._composed = opts.composed, ._composed = opts.composed,
._type_string = typ, ._type_string = typ,
._isTrusted = trusted, ._is_trusted = trusted,
}; };
return event; return event;
} }
@@ -131,9 +135,26 @@ pub fn initEvent(
self._prevent_default = false; self._prevent_default = false;
} }
pub fn acquireRef(self: *Event) void {
self._rc += 1;
}
pub fn deinit(self: *Event, shutdown: bool, page: *Page) void { pub fn deinit(self: *Event, shutdown: bool, page: *Page) void {
_ = shutdown; if (shutdown) {
page.releaseArena(self._arena); 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 { 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 { pub fn setTrusted(self: *Event) void {
self._isTrusted = true; self._is_trusted = true;
} }
pub fn setUntrusted(self: *Event) void { pub fn setUntrusted(self: *Event) void {
self._isTrusted = false; self._is_trusted = false;
} }
pub fn getIsTrusted(self: *const Event) bool { pub fn getIsTrusted(self: *const Event) bool {
return self._isTrusted; return self._is_trusted;
} }
pub fn composedPath(self: *Event, page: *Page) ![]const *EventTarget { 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) // Set isTrusted at the Event level (base of prototype chain)
if (T == Event or @hasField(T, "_isTrusted")) { if (T == Event or @hasField(T, "is_trusted")) {
self._isTrusted = trusted; self._is_trusted = trusted;
} }
} }

View File

@@ -55,7 +55,8 @@ pub fn dispatchEvent(self: *EventTarget, event: *Event, page: *Page) !bool {
if (event._event_phase != .none) { if (event._event_phase != .none) {
return error.InvalidStateError; return error.InvalidStateError;
} }
event._isTrusted = false; event._is_trusted = false;
event.acquireRef();
try page._event_manager.dispatch(self, event); try page._event_manager.dispatch(self, event);
return !event._cancelable or !event._prevent_default; return !event._cancelable or !event._prevent_default;
} }