don't allow concurrent blocking calls

This commit is contained in:
Karl Seguin
2025-08-07 08:21:53 +08:00
parent 332e264437
commit ff742c0169
4 changed files with 36 additions and 29 deletions

View File

@@ -38,6 +38,9 @@ page: *Page,
// used to prevent recursive evalution // used to prevent recursive evalution
is_evaluating: bool, is_evaluating: bool,
// used to prevent executing scripts while we're doing a blocking load
is_blocking: bool = false,
// Only once this is true can deferred scripts be run // Only once this is true can deferred scripts be run
static_scripts_done: bool, static_scripts_done: bool,
@@ -54,6 +57,7 @@ deferreds: OrderList,
shutdown: bool = false, shutdown: bool = false,
client: *HttpClient, client: *HttpClient,
allocator: Allocator, allocator: Allocator,
buffer_pool: BufferPool, buffer_pool: BufferPool,
@@ -269,6 +273,17 @@ pub fn addFromElement(self: *ScriptManager, element: *parser.Element) !void {
// http handle, these are executed without there necessarily being a free handle. // http handle, these are executed without there necessarily being a free handle.
// Thus, Http/Client.zig maintains a dedicated handle for these calls. // Thus, Http/Client.zig maintains a dedicated handle for these calls.
pub fn blockingGet(self: *ScriptManager, url: [:0]const u8) !BlockingResult { pub fn blockingGet(self: *ScriptManager, url: [:0]const u8) !BlockingResult {
std.debug.assert(self.is_blocking == false);
self.is_blocking = true;
defer {
self.is_blocking = false;
// we blocked evaluation while loading this script, there could be
// scripts ready to process.
self.evaluate();
}
var blocking = Blocking{ var blocking = Blocking{
.allocator = self.allocator, .allocator = self.allocator,
.buffer_pool = &self.buffer_pool, .buffer_pool = &self.buffer_pool,
@@ -314,6 +329,13 @@ fn evaluate(self: *ScriptManager) void {
return; return;
} }
if (self.is_blocking) {
// Cannot evaluate scripts while a blocking-load is in progress. Not
// only could that result in incorrect evaluation order, it could
// triger another blocking request, while we're doing a blocking request.
return;
}
const page = self.page; const page = self.page;
self.is_evaluating = true; self.is_evaluating = true;
defer self.is_evaluating = false; defer self.is_evaluating = false;

View File

@@ -118,13 +118,15 @@ pub const Session = struct {
std.debug.assert(self.page != null); std.debug.assert(self.page != null);
self.page.?.deinit();
self.page = null;
// RemoveJsContext() will execute the destructor of any type that // RemoveJsContext() will execute the destructor of any type that
// registered a destructor (e.g. XMLHttpRequest). // registered a destructor (e.g. XMLHttpRequest).
// Should be called before we deinit the page, because these objects
// could be referencing it.
self.executor.removeJsContext(); self.executor.removeJsContext();
self.page.?.deinit();
self.page = null;
// clear netsurf memory arena. // clear netsurf memory arena.
parser.deinit(); parser.deinit();

View File

@@ -171,12 +171,12 @@ pub const XMLHttpRequest = struct {
}; };
} }
// pub fn destructor(self: *XMLHttpRequest) void { pub fn destructor(self: *XMLHttpRequest) void {
// if (self.transfer) |transfer| { if (self.transfer) |transfer| {
// transfer.abort(); transfer.abort();
// self.transfer = null; self.transfer = null;
// } }
// } }
pub fn reset(self: *XMLHttpRequest) void { pub fn reset(self: *XMLHttpRequest) void {
self.url = null; self.url = null;

View File

@@ -74,10 +74,6 @@ transfer_pool: std.heap.MemoryPool(Transfer),
// see ScriptManager.blockingGet // see ScriptManager.blockingGet
blocking: Handle, blocking: Handle,
// Boolean to check that we don't make a blocking request while an existing
// blocking request is already being processed.
blocking_active: if (builtin.mode == .Debug) bool else void = if (builtin.mode == .Debug) false else {},
// The only place this is meant to be used is in `makeRequest` BEFORE `perform` // The only place this is meant to be used is in `makeRequest` BEFORE `perform`
// is called. It is used to generate our Cookie header. It can be used for other // is called. It is used to generate our Cookie header. It can be used for other
// purposes, but keep in mind that, while single-threaded, calls like makeRequest // purposes, but keep in mind that, while single-threaded, calls like makeRequest
@@ -199,14 +195,6 @@ pub fn request(self: *Client, req: Request) !void {
// See ScriptManager.blockingGet // See ScriptManager.blockingGet
pub fn blockingRequest(self: *Client, req: Request) !void { pub fn blockingRequest(self: *Client, req: Request) !void {
if (comptime builtin.mode == .Debug) {
std.debug.assert(self.blocking_active == false);
self.blocking_active = true;
}
defer if (comptime builtin.mode == .Debug) {
self.blocking_active = false;
};
return self.makeRequest(&self.blocking, req); return self.makeRequest(&self.blocking, req);
} }
@@ -361,7 +349,7 @@ fn endTransfer(self: *Client, transfer: *Transfer) void {
self.transfer_pool.destroy(transfer); self.transfer_pool.destroy(transfer);
errorMCheck(c.curl_multi_remove_handle(self.multi, handle.conn.easy)) catch |err| { errorMCheck(c.curl_multi_remove_handle(self.multi, handle.conn.easy)) catch |err| {
log.fatal(.http, "Failed to abort", .{ .err = err }); log.fatal(.http, "Failed to remove handle", .{ .err = err });
}; };
self.handles.release(handle); self.handles.release(handle);
@@ -372,11 +360,6 @@ fn ensureNoActiveConnection(self: *const Client) !void {
if (self.active > 0) { if (self.active > 0) {
return error.InflightConnection; return error.InflightConnection;
} }
if (comptime builtin.mode == .Debug) {
if (self.blocking_active) {
return error.InflightConnection;
}
}
} }
const Handles = struct { const Handles = struct {
@@ -429,8 +412,8 @@ const Handles = struct {
} }
fn release(self: *Handles, handle: *Handle) void { fn release(self: *Handles, handle: *Handle) void {
// client.blocking is a handle without a node, it doesn't exist in the // client.blocking is a handle without a node, it doesn't exist in
// eitehr the in_use or available lists. // either the in_use or available lists.
const node = &(handle.node orelse return); const node = &(handle.node orelse return);
self.in_use.remove(node); self.in_use.remove(node);