From f6f0e141a1af6d6852b9c0ee26448dacb260d5de Mon Sep 17 00:00:00 2001 From: Muki Kiboigo Date: Wed, 17 Sep 2025 12:11:33 -0700 Subject: [PATCH] PeristentPromiseResolver with page lifetime --- src/browser/fetch/fetch.zig | 4 +-- src/browser/streams/ReadableStream.zig | 15 +++++----- .../ReadableStreamDefaultController.zig | 3 -- .../streams/ReadableStreamDefaultReader.zig | 2 +- src/runtime/js.zig | 28 +++++++++++++++---- 5 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/browser/fetch/fetch.zig b/src/browser/fetch/fetch.zig index ee76578f..fcdc4a25 100644 --- a/src/browser/fetch/fetch.zig +++ b/src/browser/fetch/fetch.zig @@ -101,7 +101,7 @@ pub fn fetch(input: RequestInput, options: ?RequestInit, page: *Page) !Env.Promi try page.requestCookie(.{}).headersForRequest(arena, req.url, &headers); - const resolver = page.main_context.createPersistentPromiseResolver(); + const resolver = try page.main_context.createPersistentPromiseResolver(.page); const fetch_ctx = try arena.create(FetchContext); fetch_ctx.* = .{ @@ -170,7 +170,6 @@ pub fn fetch(input: RequestInput, options: ?RequestInit, page: *Page) !Env.Promi .done_callback = struct { fn doneCallback(ctx: *anyopaque) !void { const self: *FetchContext = @ptrCast(@alignCast(ctx)); - defer self.promise_resolver.deinit(); self.transfer = null; log.info(.fetch, "request complete", .{ @@ -187,7 +186,6 @@ pub fn fetch(input: RequestInput, options: ?RequestInit, page: *Page) !Env.Promi .error_callback = struct { fn errorCallback(ctx: *anyopaque, err: anyerror) void { const self: *FetchContext = @ptrCast(@alignCast(ctx)); - defer self.promise_resolver.deinit(); self.transfer = null; log.err(.fetch, "error", .{ diff --git a/src/browser/streams/ReadableStream.zig b/src/browser/streams/ReadableStream.zig index 216152da..762050a2 100644 --- a/src/browser/streams/ReadableStream.zig +++ b/src/browser/streams/ReadableStream.zig @@ -78,11 +78,15 @@ const QueueingStrategy = struct { pub fn constructor(underlying: ?UnderlyingSource, _strategy: ?QueueingStrategy, page: *Page) !*ReadableStream { const strategy: QueueingStrategy = _strategy orelse .{}; - const cancel_resolver = page.main_context.createPersistentPromiseResolver(); - const closed_resolver = page.main_context.createPersistentPromiseResolver(); + const cancel_resolver = try page.main_context.createPersistentPromiseResolver(.self); + const closed_resolver = try page.main_context.createPersistentPromiseResolver(.self); const stream = try page.arena.create(ReadableStream); - stream.* = ReadableStream{ .cancel_resolver = cancel_resolver, .closed_resolver = closed_resolver, .strategy = strategy }; + stream.* = ReadableStream{ + .cancel_resolver = cancel_resolver, + .closed_resolver = closed_resolver, + .strategy = strategy, + }; const controller = ReadableStreamDefaultController{ .stream = stream }; @@ -108,10 +112,7 @@ pub fn constructor(underlying: ?UnderlyingSource, _strategy: ?QueueingStrategy, pub fn destructor(self: *ReadableStream) void { self.cancel_resolver.deinit(); self.closed_resolver.deinit(); - - if (self.reader_resolver) |*rr| { - rr.deinit(); - } + // reader resolver is scoped to the page lifetime and is cleaned up by it. } pub fn get_locked(self: *const ReadableStream) bool { diff --git a/src/browser/streams/ReadableStreamDefaultController.zig b/src/browser/streams/ReadableStreamDefaultController.zig index c83a13a5..59ee5fb9 100644 --- a/src/browser/streams/ReadableStreamDefaultController.zig +++ b/src/browser/streams/ReadableStreamDefaultController.zig @@ -40,7 +40,6 @@ pub fn _close(self: *ReadableStreamDefaultController, _reason: ?[]const u8, page // Resolve the Reader Promise if (self.stream.reader_resolver) |*rr| { - defer rr.deinit(); try rr.resolve(ReadableStreamReadResult{ .value = .empty, .done = true }); self.stream.reader_resolver = null; } @@ -62,7 +61,6 @@ pub fn _enqueue(self: *ReadableStreamDefaultController, chunk: []const u8, page: const duped_chunk = try page.arena.dupe(u8, chunk); if (self.stream.reader_resolver) |*rr| { - defer rr.deinit(); try rr.resolve(ReadableStreamReadResult{ .value = .{ .data = duped_chunk }, .done = false }); self.stream.reader_resolver = null; } @@ -75,7 +73,6 @@ pub fn _error(self: *ReadableStreamDefaultController, err: Env.JsObject) !void { self.stream.state = .{ .errored = err }; if (self.stream.reader_resolver) |*rr| { - defer rr.deinit(); try rr.reject(err); self.stream.reader_resolver = null; } diff --git a/src/browser/streams/ReadableStreamDefaultReader.zig b/src/browser/streams/ReadableStreamDefaultReader.zig index 0708eff2..c64c2d8d 100644 --- a/src/browser/streams/ReadableStreamDefaultReader.zig +++ b/src/browser/streams/ReadableStreamDefaultReader.zig @@ -56,7 +56,7 @@ pub fn _read(self: *const ReadableStreamDefaultReader, page: *Page) !Env.Promise if (self.stream.reader_resolver) |rr| { return rr.promise(); } else { - const persistent_resolver = page.main_context.createPersistentPromiseResolver(); + const persistent_resolver = try page.main_context.createPersistentPromiseResolver(.page); self.stream.reader_resolver = persistent_resolver; return persistent_resolver.promise(); } diff --git a/src/runtime/js.zig b/src/runtime/js.zig index e18e73d6..4c4fba2e 100644 --- a/src/runtime/js.zig +++ b/src/runtime/js.zig @@ -677,6 +677,11 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { // we now simply persist every time persist() is called. js_object_list: std.ArrayListUnmanaged(PersistentObject) = .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, + // When we need to load a resource (i.e. an external script), we call // this function to get the source. This is always a reference to the // Page's fetchModuleSource, but we use a function pointer @@ -733,6 +738,10 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { p.deinit(); } + for (self.persisted_promise_resolvers.items) |*p| { + p.deinit(); + } + { var it = self.module_cache.valueIterator(); while (it.next()) |p| { @@ -1261,11 +1270,20 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { }; } - pub fn createPersistentPromiseResolver(self: *JsContext) PersistentPromiseResolver { - return .{ - .js_context = self, - .resolver = v8.Persistent(v8.PromiseResolver).init(self.isolate, v8.PromiseResolver.init(self.v8_context)), - }; + // 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. + pub fn createPersistentPromiseResolver( + self: *JsContext, + lifetime: enum { self, page }, + ) !PersistentPromiseResolver { + const resolver = v8.Persistent(v8.PromiseResolver).init(self.isolate, v8.PromiseResolver.init(self.v8_context)); + + if (lifetime == .page) { + try self.persisted_promise_resolvers.append(self.context_arena, resolver); + } + + return .{ .js_context = self, .resolver = resolver }; } // Probing is part of trying to map a JS value to a Zig union. There's