diff --git a/src/browser/Browser.zig b/src/browser/Browser.zig index 5f1257efe..c5b67033b 100644 --- a/src/browser/Browser.zig +++ b/src/browser/Browser.zig @@ -34,6 +34,7 @@ const Selector = @import("webapi/selector/Selector.zig"); const Geolocation = @import("webapi/geolocation/Geolocation.zig"); const PermissionState = @import("webapi/Permissions.zig").State; +const log = lp.log; const ArenaPool = App.ArenaPool; const Allocator = std.mem.Allocator; @@ -104,6 +105,8 @@ fc_identity_pool: std.heap.MemoryPool(js.FinalizerCallback.Identity), // #2472). frame_id_gen: u32 = 0, +foreground_task_posted: std.atomic.Value(bool) = .init(false), + const InitOpts = struct { env: js.Env.InitOpts = .{}, }; @@ -146,6 +149,8 @@ pub fn init(self: *Browser, app: *App, opts: InitOpts) !void { .heartbeat = &self.http_client.heartbeat, }; app.watchdog.register(&self.watchdog_entry); + + self.env.setForegroundTaskPostedCallback(onForegroundTaskPosted, self); } pub fn deinit(self: *Browser) void { @@ -158,6 +163,9 @@ pub fn deinit(self: *Browser) void { lp.metrics.js_heap_physical_bytes.add(-@as(i64, @intCast(self.last_reported_js_bytes))); self.last_reported_js_bytes = 0; + // V8 workers can post until the isolate is gone; http_client is freed + // after env. + self.env.setForegroundTaskPostedCallback(null, null); self.env.deinit(); // After env.deinit() the Isolate is gone, so no further weak finalizer can // fire — only now is it safe to free the pool backing their parameters. @@ -171,6 +179,24 @@ pub fn deinit(self: *Browser) void { self.selector_cache.deinit(); } +// Callback from v8 when a foreground task is posted. +// !!Can be called from various threads!! (v8 worker threads) +fn onForegroundTaskPosted(ctx: ?*anyopaque, delay_in_seconds: f64) callconv(.c) void { + if (delay_in_seconds > 0) { + return; + } + const self: *Browser = @ptrCast(@alignCast(ctx.?)); + if (self.foreground_task_posted.swap(true, .acq_rel)) { + // it was already woken up before + return; + } + + // wakeup is thread-safe + self.http_client.handles.wakeup() catch |err| { + log.err(.browser, "foreground task wakeup", .{ .err = err }); + }; +} + // Wait out a watchdog scan before clearing its termination request. pub fn prepareForTeardown(self: *Browser) void { self.app.watchdog.unregister(&self.watchdog_entry); @@ -262,15 +288,15 @@ pub fn runMicrotasks(self: *Browser) void { self.env.runMicrotasks(); } -pub fn runMacrotasks(self: *Browser) !bool { +pub fn runMacrotasks(self: *Browser) !void { const env = &self.env; try self.env.runMacrotasks(); - const ran_platform_task = env.pumpMessageLoop(); + self.foreground_task_posted.store(false, .release); + env.pumpMessageLoop(); // either of the above could have queued more microtasks env.runMicrotasks(); - return ran_platform_task; } pub fn hasBackgroundTasks(self: *Browser) bool { diff --git a/src/browser/Runner.zig b/src/browser/Runner.zig index c2fb72857..e648e89a8 100644 --- a/src/browser/Runner.zig +++ b/src/browser/Runner.zig @@ -35,7 +35,6 @@ const Runner = @This(); session: *Session, browser: *Browser, http_client: *HttpClient, -background_poll_ms: u32 = 0, pub const Opts = struct {}; @@ -224,9 +223,8 @@ fn _tick(self: *Runner, comptime is_cdp: bool, timeout_ms: u32, conditions: []Wa const has_runnable_page = hasRunnablePage(session); - var ran_platform_task = false; if (has_runnable_page) { - ran_platform_task = try browser.runMacrotasks(); + try browser.runMacrotasks(); } const activity = http_client.activity(); @@ -319,18 +317,6 @@ fn _tick(self: *Runner, comptime is_cdp: bool, timeout_ms: u32, conditions: []Wa if (has_runnable_page == false) { break :blk 200; } - if (browser.hasBackgroundTasks()) { - // if our last runMacrotasks() ran something and we now have - // a background, then don't linger in the http client waiting - // for I/O, instead, hurry back to run more tasks. - // Else, backoff to 10ms between runs. - // TODO: this is a temporary solution to ensuring background - // tasks are run promptly.The better solution is to have v8 - // wakeup the http client when there's work to do. - self.background_poll_ms = if (ran_platform_task) 0 else @min(10, @max(1, self.background_poll_ms * 2)); - // msToNextTask could be less than this, but 10ms drift is ok - break :blk self.background_poll_ms; - } break :blk browser.msToNextTask() orelse 200; }; const ms_to_wait = @min(timeout_ms, ms_to_next_task); diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index 2003b876c..dc09833e4 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -220,7 +220,7 @@ pub fn deinit(self: *Context) void { env.isolate.notifyContextDisposed(); // There can be other tasks associated with this context that we need to // purge while the context is still alive. - _ = env.pumpMessageLoop(); + env.pumpMessageLoop(); v8.v8__MicrotaskQueue__DELETE(self.microtask_queue); } diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index 799ed3b9f..edd99d786 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -567,18 +567,18 @@ pub fn msToNextTask(self: *Env) ?u64 { return if (next_task == std.math.maxInt(u64)) null else next_task; } -pub fn pumpMessageLoop(self: *const Env) bool { +pub fn pumpMessageLoop(self: *const Env) void { var hs: v8.HandleScope = undefined; v8.v8__HandleScope__CONSTRUCT(&hs, self.isolate.handle); defer v8.v8__HandleScope__DESTRUCT(&hs); const isolate = self.isolate.handle; const platform = self.platform.handle; - var ran = false; - while (v8.v8__Platform__PumpMessageLoop(platform, isolate, false)) { - ran = true; - } - return ran; + while (v8.v8__Platform__PumpMessageLoop(platform, isolate, false)) {} +} + +pub fn setForegroundTaskPostedCallback(self: *const Env, callback: v8.ForegroundTaskPostedCallback, ctx: ?*anyopaque) void { + v8.v8__Platform__SetForegroundTaskPostedCallback(self.platform.handle, self.isolate.handle, callback, ctx); } pub fn hasBackgroundTasks(self: *const Env) bool { diff --git a/src/browser/js/Local.zig b/src/browser/js/Local.zig index f56bc9a99..1dc926183 100644 --- a/src/browser/js/Local.zig +++ b/src/browser/js/Local.zig @@ -121,7 +121,7 @@ pub fn newCallback( pub fn runMacrotasks(self: *const Local) void { const env = self.ctx.env; - _ = env.pumpMessageLoop(); + env.pumpMessageLoop(); env.runMicrotasks(); // macrotasks can cause microtasks to queue }