PeristentPromiseResolver with page lifetime

Author: mookums

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", .{

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 {

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;
     }

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();
                 }

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