Skip to content

Commit afa0d5b

Browse files
committed
Try fixing segfault by not being as aggressive with freeing Persisted Resolvers
1 parent 4d1e416 commit afa0d5b

File tree

4 files changed

+15
-11
lines changed

4 files changed

+15
-11
lines changed

src/browser/fetch/fetch.zig

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ pub fn fetch(input: RequestInput, options: ?RequestInit, page: *Page) !Env.Promi
101101

102102
try page.requestCookie(.{}).headersForRequest(arena, req.url, &headers);
103103

104-
const resolver = page.main_context.createPersistentPromiseResolver();
104+
const resolver = try page.main_context.createPersistentPromiseResolver();
105105

106106
const fetch_ctx = try arena.create(FetchContext);
107107
fetch_ctx.* = .{
@@ -170,7 +170,6 @@ pub fn fetch(input: RequestInput, options: ?RequestInit, page: *Page) !Env.Promi
170170
.done_callback = struct {
171171
fn doneCallback(ctx: *anyopaque) !void {
172172
const self: *FetchContext = @ptrCast(@alignCast(ctx));
173-
defer self.promise_resolver.deinit();
174173
self.transfer = null;
175174

176175
log.info(.fetch, "request complete", .{
@@ -187,7 +186,6 @@ pub fn fetch(input: RequestInput, options: ?RequestInit, page: *Page) !Env.Promi
187186
.error_callback = struct {
188187
fn errorCallback(ctx: *anyopaque, err: anyerror) void {
189188
const self: *FetchContext = @ptrCast(@alignCast(ctx));
190-
defer self.promise_resolver.deinit();
191189
self.transfer = null;
192190

193191
log.err(.fetch, "error", .{

src/browser/streams/ReadableStream.zig

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,8 @@ const QueueingStrategy = struct {
7878
pub fn constructor(underlying: ?UnderlyingSource, _strategy: ?QueueingStrategy, page: *Page) !*ReadableStream {
7979
const strategy: QueueingStrategy = _strategy orelse .{};
8080

81-
const cancel_resolver = page.main_context.createPersistentPromiseResolver();
82-
const closed_resolver = page.main_context.createPersistentPromiseResolver();
81+
const cancel_resolver = try page.main_context.createPersistentPromiseResolver();
82+
const closed_resolver = try page.main_context.createPersistentPromiseResolver();
8383

8484
const stream = try page.arena.create(ReadableStream);
8585
stream.* = ReadableStream{ .cancel_resolver = cancel_resolver, .closed_resolver = closed_resolver, .strategy = strategy };
@@ -106,9 +106,6 @@ pub fn constructor(underlying: ?UnderlyingSource, _strategy: ?QueueingStrategy,
106106
}
107107

108108
pub fn destructor(self: *ReadableStream) void {
109-
self.cancel_resolver.deinit();
110-
self.closed_resolver.deinit();
111-
112109
if (self.reader_resolver) |*rr| {
113110
rr.deinit();
114111
}

src/browser/streams/ReadableStreamDefaultReader.zig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ pub fn _read(self: *const ReadableStreamDefaultReader, page: *Page) !Env.Promise
5656
if (self.stream.reader_resolver) |rr| {
5757
return rr.promise();
5858
} else {
59-
const persistent_resolver = page.main_context.createPersistentPromiseResolver();
59+
const persistent_resolver = try page.main_context.createPersistentPromiseResolver();
6060
self.stream.reader_resolver = persistent_resolver;
6161
return persistent_resolver.promise();
6262
}

src/runtime/js.zig

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,9 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
677677
// we now simply persist every time persist() is called.
678678
js_object_list: std.ArrayListUnmanaged(PersistentObject) = .empty,
679679

680+
681+
persisted_promise_resolvers: std.ArrayListUnmanaged(v8.Persistent(v8.PromiseResolver)) = .empty,
682+
680683
// When we need to load a resource (i.e. an external script), we call
681684
// this function to get the source. This is always a reference to the
682685
// Page's fetchModuleSource, but we use a function pointer
@@ -733,6 +736,10 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
733736
p.deinit();
734737
}
735738

739+
for (self.persisted_promise_resolvers.items) |*p| {
740+
p.deinit();
741+
}
742+
736743
{
737744
var it = self.module_cache.valueIterator();
738745
while (it.next()) |p| {
@@ -1261,10 +1268,12 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
12611268
};
12621269
}
12631270

1264-
pub fn createPersistentPromiseResolver(self: *JsContext) PersistentPromiseResolver {
1271+
pub fn createPersistentPromiseResolver(self: *JsContext) !PersistentPromiseResolver {
1272+
const resolver = v8.Persistent(v8.PromiseResolver).init(self.isolate, v8.PromiseResolver.init(self.v8_context));
1273+
try self.persisted_promise_resolvers.append(self.context_arena, resolver);
12651274
return .{
12661275
.js_context = self,
1267-
.resolver = v8.Persistent(v8.PromiseResolver).init(self.isolate, v8.PromiseResolver.init(self.v8_context)),
1276+
.resolver = resolver,
12681277
};
12691278
}
12701279

0 commit comments

Comments
 (0)