From 01ecd725b8debc3a38805570d66e483c1b1a7e0f Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 1 Jan 2026 09:57:41 +0800 Subject: [PATCH] cleanup resolvers --- src/browser/js/Context.zig | 114 +++++++----------- src/browser/js/Isolate.zig | 4 + src/browser/js/Object.zig | 4 +- src/browser/js/PromiseResolver.zig | 16 ++- src/browser/js/Script.zig | 38 ------ src/browser/js/Snapshot.zig | 2 + src/browser/js/Value.zig | 4 + src/browser/js/js.zig | 1 - src/browser/webapi/CustomElementRegistry.zig | 4 +- src/browser/webapi/animation/Animation.zig | 4 +- src/browser/webapi/navigation/Navigation.zig | 4 +- src/browser/webapi/net/Fetch.zig | 4 +- src/browser/webapi/streams/ReadableStream.zig | 4 +- .../ReadableStreamDefaultController.zig | 4 +- 14 files changed, 83 insertions(+), 124 deletions(-) delete mode 100644 src/browser/js/Script.zig diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index b164bfb2..5f19b101 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -34,8 +34,11 @@ const Allocator = std.mem.Allocator; const PersistentObject = v8.Persistent(v8.Object); const PersistentModule = v8.Persistent(v8.Module); const PersistentPromise = v8.Persistent(v8.Promise); +const PersistentPromiseResolver = v8.Persistent(v8.PromiseResolver); const TaggedAnyOpaque = js.TaggedAnyOpaque; +const IS_DEBUG = builtin.mode == .Debug; + // Loosely maps to a Browser Page. const Context = @This(); @@ -70,6 +73,8 @@ call_depth: usize = 0, // The key is the @intFromPtr of the Zig value identity_map: std.AutoHashMapUnmanaged(usize, PersistentObject) = .empty, +persisted_promise_resolvers: std.ArrayList(PersistentPromiseResolver) = .empty, + // Some web APIs have to manage opaque values. Ideally, they use an // js.Object, but the js.Object has no lifetime guarantee beyond the // current call. They can call .persist() on their js.Object to get @@ -81,11 +86,7 @@ identity_map: std.AutoHashMapUnmanaged(usize, PersistentObject) = .empty, global_values: std.ArrayList(js.Global(js.Value)) = .empty, global_objects: std.ArrayList(js.Global(js.Object)) = .empty, global_functions: std.ArrayList(js.Global(js.Function)) = .empty, - -// Various web APIs depend on having a persistent promise resolver. They -// require for this PromiseResolver to be valid for a lifetime longer than -// the function that resolves/rejects them. -persisted_promise_resolvers: std.ArrayListUnmanaged(v8.Persistent(v8.PromiseResolver)) = .empty, +global_promise_resolvers: std.ArrayList(js.Global(js.PromiseResolver)) = .empty, // Some Zig types have code to execute to cleanup destructor_callbacks: std.ArrayListUnmanaged(DestructorCallback) = .empty, @@ -171,6 +172,10 @@ pub fn deinit(self: *Context) void { global.deinit(); } + for (self.global_promise_resolvers.items) |*global| { + global.deinit(); + } + for (self.persisted_promise_resolvers.items) |*p| { p.deinit(); } @@ -205,16 +210,7 @@ pub fn eval(self: *Context, src: []const u8, name: ?[]const u8) !void { } pub fn exec(self: *Context, src: []const u8, name: ?[]const u8) !js.Value { - const v8_context = v8.Context{ .handle = self.handle }; - const v8_isolate = v8.Isolate{ .handle = self.isolate.handle }; - - const scr = try compileScript(v8_isolate, v8_context, src, name); - - const value = scr.run(v8_context.handle) catch { - return error.ExecutionError; - }; - - return self.createValue(value); + return self.compileAndRun(src, name); } pub fn module(self: *Context, comptime want_result: bool, src: []const u8, url: []const u8, cacheable: bool) !(if (want_result) ModuleEntry else void) { @@ -315,16 +311,11 @@ pub fn stringToFunction(self: *Context, str: []const u8) !js.Function { } const full = try std.fmt.allocPrintSentinel(self.call_arena, "(function(e) {{ {s}{s} }})", .{ normalized, extra }, 0); - const v8_context = v8.Context{ .handle = self.handle }; - const v8_isolate = v8.Isolate{ .handle = self.isolate.handle }; - const script = try compileScript(v8_isolate, v8_context, full, null); - const js_value = script.run(v8_context.handle) catch { - return error.ExecutionError; - }; + const js_value = try self.compileAndRun(full, null); if (!js_value.isFunction()) { return error.StringFunctionError; } - return self.createFunction(js_value); + return self.createFunction(.{ .handle = js_value.handle }); } // After we compile a module, whether it's a top-level one, or a nested one, @@ -390,7 +381,9 @@ pub fn createObject(self: *Context, js_value: v8.Value) js.Object { pub fn createFunction(self: *Context, js_value: v8.Value) !js.Function { // caller should have made sure this was a function - std.debug.assert(js_value.isFunction()); + if (comptime IS_DEBUG) { + std.debug.assert(js_value.isFunction()); + } return .{ .ctx = self, @@ -398,6 +391,13 @@ pub fn createFunction(self: *Context, js_value: v8.Value) !js.Function { }; } +pub fn newString(self: *Context, str: []const u8) js.String { + return .{ + .ctx = self, + .handle = self.isolate.newStringHandle(str), + }; +} + pub fn throw(self: *Context, err: []const u8) js.Exception { const v8_isolate = v8.Isolate{ .handle = self.isolate.handle }; const js_value = js._createException(v8_isolate, err); @@ -1203,38 +1203,8 @@ pub fn runMicrotasks(self: *Context) void { self.isolate.performMicrotasksCheckpoint(); } -// creates a PersistentPromiseResolver, taking in a lifetime parameter. -// If the lifetime is page, the page will clean up the PersistentPromiseResolver. -// If the lifetime is self, you will be expected to deinitalize the PersistentPromiseResolver. -const PromiseResolverLifetime = enum { - none, - self, // it's a persisted promise, but it'll be managed by the caller - page, // it's a persisted promise, tied to the page lifetime -}; -fn PromiseResolverType(comptime lifetime: PromiseResolverLifetime) type { - if (lifetime == .none) { - return js.PromiseResolver; - } - return error{OutOfMemory}!js.PersistentPromiseResolver; -} -pub fn createPromiseResolver(self: *Context, comptime lifetime: PromiseResolverLifetime) PromiseResolverType(lifetime) { - if (comptime lifetime == .none) { - return js.PromiseResolver.init(self); - } - - const v8_context = v8.Context{ .handle = self.handle }; - const resolver = v8.PromiseResolver.init(v8_context); - const v8_isolate = v8.Isolate{ .handle = self.isolate.handle }; - const persisted = v8.Persistent(v8.PromiseResolver).init(v8_isolate, resolver); - - if (comptime lifetime == .page) { - try self.persisted_promise_resolvers.append(self.arena, persisted); - } - - return .{ - .context = self, - .resolver = persisted, - }; +pub fn createPromiseResolver(self: *Context) js.PromiseResolver { + return js.PromiseResolver.init(self); } // == Callbacks == @@ -1928,24 +1898,30 @@ fn jsUnsignedIntToZig(comptime T: type, max: comptime_int, maybe: u32) !T { return error.InvalidArgument; } -fn compileScript(isolate: v8.Isolate, ctx: v8.Context, src: []const u8, name: ?[]const u8) !js.Script { - // compile - const script_name = v8.String.initUtf8(isolate, name orelse "anonymous"); - const script_source = v8.String.initUtf8(isolate, src); +fn compileAndRun(self: *Context, src: []const u8, name: ?[]const u8) !js.Value { + const script_name = self.isolate.newStringHandle(name orelse "anonymous"); + const script_source = self.isolate.newStringHandle(src); - const origin = v8.ScriptOrigin.initDefault(script_name.toValue()); + // Create ScriptOrigin + var origin: v8.c.ScriptOrigin = undefined; + v8.c.v8__ScriptOrigin__CONSTRUCT(&origin, @ptrCast(script_name)); - var script_comp_source: v8.ScriptCompilerSource = undefined; - v8.ScriptCompilerSource.init(&script_comp_source, script_source, origin, null); - defer script_comp_source.deinit(); + // Create ScriptCompilerSource + var script_comp_source: v8.c.ScriptCompilerSource = undefined; + v8.c.v8__ScriptCompiler__Source__CONSTRUCT2(script_source, &origin, null, &script_comp_source); + defer v8.c.v8__ScriptCompiler__Source__DESTRUCT(&script_comp_source); - const v8_script = v8.ScriptCompiler.compile( - ctx, + // Compile the script + const v8_script = v8.c.v8__ScriptCompiler__Compile( + self.handle, &script_comp_source, - .kNoCompileOptions, - .kNoCacheNoReason, - ) catch return error.CompilationError; - return .{ .handle = v8_script.handle }; + v8.c.kNoCompileOptions, + v8.c.kNoCacheNoReason, + ) orelse return error.CompilationError; + + // Run the script + const result = v8.c.v8__Script__Run(v8_script, self.handle) orelse return error.ExecutionError; + return .{ .ctx = self, .handle = result }; } fn compileModule(isolate: v8.Isolate, src: []const u8, name: []const u8) !js.Module { diff --git a/src/browser/js/Isolate.zig b/src/browser/js/Isolate.zig index a2d865f1..fa570947 100644 --- a/src/browser/js/Isolate.zig +++ b/src/browser/js/Isolate.zig @@ -63,3 +63,7 @@ pub fn throwException(self: Isolate, value: anytype) v8.Value { .handle = v8.c.v8__Isolate__ThrowException(self.handle, handle).?, }; } + +pub fn newStringHandle(self: Isolate, str: []const u8) *const v8.c.String { + return v8.c.v8__String__NewFromUtf8(self.handle, str.ptr, v8.c.kNormal, @as(c_int, @intCast(str.len))).?; +} diff --git a/src/browser/js/Object.zig b/src/browser/js/Object.zig index 5b62474f..1cd6a14d 100644 --- a/src/browser/js/Object.zig +++ b/src/browser/js/Object.zig @@ -68,7 +68,7 @@ pub fn set(self: Object, key: []const u8, value: anytype, opts: SetOpts) error{ pub fn get(self: Object, key: []const u8) !js.Value { const ctx = self.ctx; - const js_key = v8.c.v8__String__NewFromUtf8(ctx.isolate.handle, key.ptr, v8.c.kNormal, @intCast(key.len)).?; + const js_key = ctx.isolate.newStringHandle(key); const js_val_handle = v8.c.v8__Object__Get(self.handle, ctx.handle, js_key) orelse return error.JsException; const js_val = v8.Value{ .handle = js_val_handle }; return ctx.createValue(js_val); @@ -119,7 +119,7 @@ pub fn getFunction(self: Object, name: []const u8) !?js.Function { } const ctx = self.ctx; - const js_name = v8.c.v8__String__NewFromUtf8(ctx.isolate.handle, name.ptr, v8.c.kNormal, @intCast(name.len)).?; + const js_name = ctx.isolate.newStringHandle(name); const js_val_handle = v8.c.v8__Object__Get(self.handle, ctx.handle, js_name) orelse return error.JsException; const js_value = v8.Value{ .handle = js_val_handle }; diff --git a/src/browser/js/PromiseResolver.zig b/src/browser/js/PromiseResolver.zig index 583bf39c..ee6d7945 100644 --- a/src/browser/js/PromiseResolver.zig +++ b/src/browser/js/PromiseResolver.zig @@ -22,10 +22,10 @@ const log = @import("../../log.zig"); const PromiseResolver = @This(); -ctx: *const js.Context, +ctx: *js.Context, handle: *const v8.c.PromiseResolver, -pub fn init(ctx: *const js.Context) PromiseResolver { +pub fn init(ctx: *js.Context) PromiseResolver { return .{ .ctx = ctx, .handle = v8.c.v8__Promise__Resolver__New(ctx.handle).?, @@ -73,3 +73,15 @@ fn _reject(self: PromiseResolver, value: anytype) !void { } ctx.runMicrotasks(); } + +pub fn persist(self: PromiseResolver) !PromiseResolver { + var ctx = self.ctx; + + const global = js.Global(PromiseResolver).init(ctx.isolate.handle, self.handle); + try ctx.global_promise_resolvers.append(ctx.arena, global); + + return .{ + .ctx = ctx, + .handle = global.local(), + }; +} diff --git a/src/browser/js/Script.zig b/src/browser/js/Script.zig deleted file mode 100644 index 8b9c5676..00000000 --- a/src/browser/js/Script.zig +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (C) 2023-2025 Lightpanda (Selecy SAS) -// -// Francis Bouvier -// Pierre Tachoire -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU Affero General Public License as -// published by the Free Software Foundation, either version 3 of the -// License, or (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Affero General Public License for more details. -// -// You should have received a copy of the GNU Affero General Public License -// along with this program. If not, see . - -const js = @import("js.zig"); -const v8 = js.v8; - -const Script = @This(); - -handle: *const v8.c.Script, - -pub fn compile(ctx_handle: *const v8.c.Context, src_handle: *const v8.c.String, origin: ?*const v8.c.ScriptOrigin) !Script { - if (v8.c.v8__Script__Compile(ctx_handle, src_handle, origin)) |handle| { - return .{ .handle = handle }; - } - return error.JsException; -} - -pub fn run(self: Script, ctx_handle: *const v8.c.Context) !v8.Value { - if (v8.c.v8__Script__Run(self.handle, ctx_handle)) |value| { - return .{ .handle = value }; - } - return error.JsException; -} diff --git a/src/browser/js/Snapshot.zig b/src/browser/js/Snapshot.zig index 4c49079e..5fe8b50a 100644 --- a/src/browser/js/Snapshot.zig +++ b/src/browser/js/Snapshot.zig @@ -526,8 +526,10 @@ fn protoIndexLookup(comptime JsApi: type) ?bridge.JsApiLookup.BackingInt { fn illegalConstructorCallback(raw_info: ?*const v8.c.FunctionCallbackInfo) callconv(.c) void { const isolate = v8.c.v8__FunctionCallbackInfo__GetIsolate(raw_info); log.warn(.js, "Illegal constructor call", .{}); + const message = v8.c.v8__String__NewFromUtf8(isolate, "Illegal Constructor", v8.c.kNormal, 19); const js_exception = v8.c.v8__Exception__TypeError(message); + _ = v8.c.v8__Isolate__ThrowException(isolate, js_exception); var return_value: v8.c.ReturnValue = undefined; v8.c.v8__FunctionCallbackInfo__GetReturnValue(raw_info, &return_value); diff --git a/src/browser/js/Value.zig b/src/browser/js/Value.zig index b9180d66..5e60c6bc 100644 --- a/src/browser/js/Value.zig +++ b/src/browser/js/Value.zig @@ -54,6 +54,10 @@ pub fn isSymbol(self: Value) bool { return v8.c.v8__Value__IsSymbol(self.handle); } +pub fn isFunction(self: Value) bool { + return v8.c.v8__Value__IsFunction(self.handle); +} + pub fn toString(self: Value, opts: js.String.ToZigOpts) ![]u8 { return self._toString(false, opts); } diff --git a/src/browser/js/js.zig b/src/browser/js/js.zig index 8b9680db..77b89a15 100644 --- a/src/browser/js/js.zig +++ b/src/browser/js/js.zig @@ -42,7 +42,6 @@ pub const PromiseResolver = @import("PromiseResolver.zig"); pub const Module = @import("Module.zig"); pub const BigInt = @import("BigInt.zig"); pub const Name = @import("Name.zig"); -pub const Script = @import("Script.zig"); pub const Integer = @import("Integer.zig"); pub const Global = @import("global.zig").Global; diff --git a/src/browser/webapi/CustomElementRegistry.zig b/src/browser/webapi/CustomElementRegistry.zig index 70c67bef..749f14a4 100644 --- a/src/browser/webapi/CustomElementRegistry.zig +++ b/src/browser/webapi/CustomElementRegistry.zig @@ -30,7 +30,7 @@ const CustomElementDefinition = @import("CustomElementDefinition.zig"); const CustomElementRegistry = @This(); _definitions: std.StringHashMapUnmanaged(*CustomElementDefinition) = .{}, -_when_defined: std.StringHashMapUnmanaged(js.PersistentPromiseResolver) = .{}, +_when_defined: std.StringHashMapUnmanaged(js.PromiseResolver) = .{}, const DefineOptions = struct { extends: ?[]const u8 = null, @@ -131,7 +131,7 @@ pub fn whenDefined(self: *CustomElementRegistry, name: []const u8, page: *Page) errdefer _ = self._when_defined.remove(name); const owned_name = try page.dupeString(name); - const resolver = try page.js.createPromiseResolver(.page); + const resolver = try page.js.createPromiseResolver().persist(); gop.key_ptr.* = owned_name; gop.value_ptr.* = resolver; diff --git a/src/browser/webapi/animation/Animation.zig b/src/browser/webapi/animation/Animation.zig index f9760c1e..f9a7988f 100644 --- a/src/browser/webapi/animation/Animation.zig +++ b/src/browser/webapi/animation/Animation.zig @@ -46,7 +46,7 @@ pub fn getPending(_: *const Animation) bool { pub fn getFinished(self: *Animation, page: *Page) !js.Promise { if (self._finished_resolver == null) { - const resolver = page.js.createPromiseResolver(.none); + const resolver = try page.js.createPromiseResolver().persist(); resolver.resolve("Animation.getFinished", self); self._finished_resolver = resolver; } @@ -56,7 +56,7 @@ pub fn getFinished(self: *Animation, page: *Page) !js.Promise { pub fn getReady(self: *Animation, page: *Page) !js.Promise { // never resolved, because we're always "finished" if (self._ready_resolver == null) { - const resolver = page.js.createPromiseResolver(.none); + const resolver = try page.js.createPromiseResolver().persist(); self._ready_resolver = resolver; } return self._ready_resolver.?.promise(); diff --git a/src/browser/webapi/navigation/Navigation.zig b/src/browser/webapi/navigation/Navigation.zig index 6fc5e65e..e1afa334 100644 --- a/src/browser/webapi/navigation/Navigation.zig +++ b/src/browser/webapi/navigation/Navigation.zig @@ -265,8 +265,8 @@ pub fn navigateInner( // // These will only settle on same-origin navigation (mostly intended for SPAs). // It is fine (and expected) for these to not settle on cross-origin requests :) - const committed = try page.js.createPromiseResolver(.page); - const finished = try page.js.createPromiseResolver(.page); + const committed = try page.js.createPromiseResolver().persist(); + const finished = try page.js.createPromiseResolver().persist(); const new_url = try URL.resolve(arena, page.url, url, .{}); const is_same_document = URL.eqlDocument(new_url, page.url); diff --git a/src/browser/webapi/net/Fetch.zig b/src/browser/webapi/net/Fetch.zig index 81d30295..7a2d7f33 100644 --- a/src/browser/webapi/net/Fetch.zig +++ b/src/browser/webapi/net/Fetch.zig @@ -36,7 +36,7 @@ _page: *Page, _url: []const u8, _buf: std.ArrayList(u8), _response: *Response, -_resolver: js.PersistentPromiseResolver, +_resolver: js.PromiseResolver, pub const Input = Request.Input; pub const InitOpts = Request.InitOpts; @@ -49,7 +49,7 @@ pub fn init(input: Input, options: ?InitOpts, page: *Page) !js.Promise { ._page = page, ._buf = .empty, ._url = try page.arena.dupe(u8, request._url), - ._resolver = try page.js.createPromiseResolver(.page), + ._resolver = try page.js.createPromiseResolver().persist(), ._response = try Response.init(null, .{ .status = 0 }, page), }; diff --git a/src/browser/webapi/streams/ReadableStream.zig b/src/browser/webapi/streams/ReadableStream.zig index ed1d0149..98e326f5 100644 --- a/src/browser/webapi/streams/ReadableStream.zig +++ b/src/browser/webapi/streams/ReadableStream.zig @@ -182,7 +182,7 @@ pub fn cancel(self: *ReadableStream, reason: ?[]const u8, page: *Page) !js.Promi var c = &self._cancel.?; if (c.resolver == null) { - c.resolver = try page.js.createPromiseResolver(.self); + c.resolver = try page.js.createPromiseResolver().persist(); } // Execute the cancel callback if provided @@ -213,7 +213,7 @@ pub fn cancel(self: *ReadableStream, reason: ?[]const u8, page: *Page) !js.Promi const Cancel = struct { callback: ?js.Function = null, reason: ?[]const u8 = null, - resolver: ?js.PersistentPromiseResolver = null, + resolver: ?js.PromiseResolver = null, }; pub const JsApi = struct { diff --git a/src/browser/webapi/streams/ReadableStreamDefaultController.zig b/src/browser/webapi/streams/ReadableStreamDefaultController.zig index 6de3505a..72956f06 100644 --- a/src/browser/webapi/streams/ReadableStreamDefaultController.zig +++ b/src/browser/webapi/streams/ReadableStreamDefaultController.zig @@ -42,7 +42,7 @@ _page: *Page, _stream: *ReadableStream, _arena: std.mem.Allocator, _queue: std.ArrayList(Chunk), -_pending_reads: std.ArrayList(js.PersistentPromiseResolver), +_pending_reads: std.ArrayList(js.PromiseResolver), _high_water_mark: u32, pub fn init(stream: *ReadableStream, high_water_mark: u32, page: *Page) !*ReadableStreamDefaultController { @@ -57,7 +57,7 @@ pub fn init(stream: *ReadableStream, high_water_mark: u32, page: *Page) !*Readab } pub fn addPendingRead(self: *ReadableStreamDefaultController, page: *Page) !js.Promise { - const resolver = try page.js.createPromiseResolver(.page); + const resolver = try page.js.createPromiseResolver().persist(); try self._pending_reads.append(self._arena, resolver); return resolver.promise(); }