From 995efd57e62730728d4c169ee14e4edd4b3ce4a8 Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Fri, 5 Jun 2026 17:32:21 +0800
Subject: [PATCH] Fix two arena leaks

The first is with WGS not flushing the deferring layer after its synchronous
importScripts call, see: https://github.com/lightpanda-io/browser/pull/2329#discussion_r3271068955

The second is from the ability of an XHR request to be re-used. This was gated
on a boolean, but the ordering means that the 1st requests' released gets
blocked by the gate, and thus we're always 1 release short. The solution is to
use a counter instead of a boolean.
---
 src/browser/tests/net/xhr.html                | 32 +++++++++++++++++++
 src/browser/tests/worker/defer-noop.js        |  3 ++
 .../tests/worker/fetch-defer-worker.js        | 20 ++++++++++++
 src/browser/tests/worker/worker.html          | 18 +++++++++++
 src/browser/webapi/WorkerGlobalScope.zig      |  2 ++
 src/browser/webapi/net/XMLHttpRequest.zig     | 11 ++++---
 6 files changed, 82 insertions(+), 4 deletions(-)
 create mode 100644 src/browser/tests/worker/defer-noop.js
 create mode 100644 src/browser/tests/worker/fetch-defer-worker.js

diff --git a/src/browser/tests/net/xhr.html b/src/browser/tests/net/xhr.html
index 3b23348d..fff72d38 100644
--- a/src/browser/tests/net/xhr.html
+++ b/src/browser/tests/net/xhr.html
@@ -431,6 +431,38 @@
   }
 </script>
 
+<script id=xhr_reuse type=module>
+  {
+    // Reusing the same XHR object from its own onload handler (open()+send()
+    // again) must not leak: send() acquires a self-ref and each request's
+    // terminal callback releases one. The second send() starts while the
+    // first request's release is still pending (it runs after this load
+    // dispatch), so both refs are briefly live. A regression here is caught
+    // by the test runner's ArenaPool leak check at teardown.
+    const state = await testing.async();
+
+    const req = new XMLHttpRequest();
+    let loads = 0;
+    req.onload = () => {
+      loads += 1;
+      if (loads === 1) {
+        // reuse the same object for a second request
+        req.open('GET', 'http://127.0.0.1:9582/xhr');
+        req.send();
+      } else {
+        state.resolve();
+      }
+    };
+    req.open('GET', 'http://127.0.0.1:9582/xhr');
+    req.send();
+
+    await state.done(() => {
+      testing.expectEqual(2, loads);
+      testing.expectEqual(200, req.status);
+    });
+  }
+</script>
+
 <script id=xhr_upload type=module>
   {
     // upload is an XMLHttpRequestUpload (an XMLHttpRequestEventTarget) and
diff --git a/src/browser/tests/worker/defer-noop.js b/src/browser/tests/worker/defer-noop.js
new file mode 100644
index 00000000..2b8133c4
--- /dev/null
+++ b/src/browser/tests/worker/defer-noop.js
@@ -0,0 +1,3 @@
+// Helper for fetch-defer-worker.js: a trivial 200-OK script body that gives
+// importScripts() a blocking syncRequest to run (its contents don't matter).
+var deferNoop = 1;
diff --git a/src/browser/tests/worker/fetch-defer-worker.js b/src/browser/tests/worker/fetch-defer-worker.js
new file mode 100644
index 00000000..01e98e5b
--- /dev/null
+++ b/src/browser/tests/worker/fetch-defer-worker.js
@@ -0,0 +1,20 @@
+// Regression for the worker deferred-fetch leak: a fetch() whose response
+// arrives while a blocking importScripts() syncRequest is in flight gets
+// deferred by the DeferringLayer. A worker has no ScriptManager to flush it,
+// so without WorkerGlobalScope.importScript's flushFrame the deferred fetch
+// never resolves (this await hangs) and its Response arena leaks on teardown.
+//
+// Ordering matters: fetch() and importScripts() run synchronously with no
+// event-loop pump between them, so the fetch is in-flight (not yet completed)
+// when importScripts begins pumping ticks — its done lands while the worker's
+// frame still has a blocking request, which is exactly what triggers deferral.
+self.onmessage = async function() {
+  const pending = fetch('http://127.0.0.1:9582/xhr');
+  importScripts('defer-noop.js');
+  try {
+    const response = await pending;
+    postMessage({ ok: true, status: response.status });
+  } catch (e) {
+    postMessage({ ok: false, err: String(e) });
+  }
+};
diff --git a/src/browser/tests/worker/worker.html b/src/browser/tests/worker/worker.html
index 881c6b6c..56c23dbd 100644
--- a/src/browser/tests/worker/worker.html
+++ b/src/browser/tests/worker/worker.html
@@ -357,6 +357,24 @@
   }
 </script>
 
+<script id="fetch_deferred_behind_importScripts" type=module>
+  // A worker fetch() whose response arrives during a blocking importScripts()
+  // is deferred by the DeferringLayer; the worker must flush it (it has no
+  // ScriptManager). Without the flush the fetch never resolves and its
+  // Response arena leaks (caught by the runner's ArenaPool leak check).
+  {
+    const state = await testing.async();
+    const worker = new Worker('fetch-defer-worker.js');
+    worker.onmessage = (event) => state.resolve(event.data);
+    worker.postMessage(null);
+
+    await state.done((data) => {
+      testing.expectTrue(data.ok, 'worker fetch failed: ' + data.err);
+      testing.expectEqual(200, data.status);
+    });
+  }
+</script>
+
 <script id="worker_post_before_load" type=module>
   // Per spec, messages posted before the worker has finished loading must
   // be buffered and delivered once onmessage is registered. Without the
diff --git a/src/browser/webapi/WorkerGlobalScope.zig b/src/browser/webapi/WorkerGlobalScope.zig
index 2955777b..1cb0927c 100644
--- a/src/browser/webapi/WorkerGlobalScope.zig
+++ b/src/browser/webapi/WorkerGlobalScope.zig
@@ -476,6 +476,8 @@ fn importScript(self: *WorkerGlobalScope, arena: Allocator, url: [:0]const u8) !
         return error.NetworkError;
     }
 
+    defer http_client.deferring_layer.flushFrame(self._frame_id);
+
     var ls: JS.Local.Scope = undefined;
     self.js.localScope(&ls);
     defer ls.deinit();
diff --git a/src/browser/webapi/net/XMLHttpRequest.zig b/src/browser/webapi/net/XMLHttpRequest.zig
index 569959d7..4e2d5f46 100644
--- a/src/browser/webapi/net/XMLHttpRequest.zig
+++ b/src/browser/webapi/net/XMLHttpRequest.zig
@@ -48,7 +48,10 @@ _proto: *XMLHttpRequestEventTarget,
 _upload: ?*XMLHttpRequestUpload = null,
 _arena: Allocator,
 _http_response: ?HttpClient.Response = null,
-_active_request: bool = false,
+
+// number of inflight requests, we can have multiple, e.g. xhr calling its own
+// send from the onload callback
+_active_requests: u8 = 0,
 
 _url: [:0]const u8 = "",
 _method: http.Method = .GET,
@@ -124,10 +127,10 @@ pub fn deinit(self: *XMLHttpRequest, page: *Page) void {
 }
 
 fn releaseSelfRef(self: *XMLHttpRequest) void {
-    if (self._active_request == false) {
+    if (self._active_requests == 0) {
         return;
     }
-    self._active_request = false;
+    self._active_requests -= 1;
     self.releaseRef(self._exec.page);
 }
 
@@ -255,7 +258,7 @@ pub fn send(self: *XMLHttpRequest, body_: ?BodyInit, exec_: *const Execution) !v
     }
 
     self.acquireRef();
-    self._active_request = true;
+    self._active_requests += 1;
 
     exec.makeRequest(.{
         .ctx = self,