From 66a19b85501dad8c0bfc5577b8d0b835877ad0dc Mon Sep 17 00:00:00 2001
From: Andreas Kling <andreas@ladybird.org>
Date: Tue, 29 Jul 2025 21:23:30 +0200
Subject: [PATCH] LibWeb: Make ESO "fetch group" weakly reference the fetch
 records

Otherwise we end up holding on to every fetch record indefinitely.

Found by analyzing GC heap graphs on Discord.
---
 Libraries/LibWeb/Fetch/Fetching/Fetching.cpp          | 6 +++---
 Libraries/LibWeb/Fetch/Infrastructure/FetchRecord.cpp | 7 +++++++
 Libraries/LibWeb/Fetch/Infrastructure/FetchRecord.h   | 9 +++++++--
 Libraries/LibWeb/Fetch/Infrastructure/HTTP/Requests.h | 1 -
 Libraries/LibWeb/HTML/Scripting/Environments.cpp      | 1 -
 Libraries/LibWeb/HTML/Scripting/Environments.h        | 5 +++--
 6 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/Libraries/LibWeb/Fetch/Fetching/Fetching.cpp b/Libraries/LibWeb/Fetch/Fetching/Fetching.cpp
index 389c3dc15fd..b270aab9b27 100644
--- a/Libraries/LibWeb/Fetch/Fetching/Fetching.cpp
+++ b/Libraries/LibWeb/Fetch/Fetching/Fetching.cpp
@@ -1789,9 +1789,9 @@ WebIDL::ExceptionOr<GC::Ref<PendingResponse>> http_network_or_cache_fetch(JS::Re
             auto& group = http_request->client()->fetch_group();
 
             // 3. Let inflightRecords be the set of fetch records in group whose request’s keepalive is true and done flag is unset.
-            Vector<GC::Ref<Infrastructure::FetchRecord>> in_flight_records;
-            for (auto const& fetch_record : group) {
-                if (fetch_record->request()->keepalive() && !fetch_record->request()->done())
+            GC::RootVector<GC::Ref<Infrastructure::FetchRecord>> in_flight_records(vm.heap());
+            for (auto& fetch_record : group) {
+                if (fetch_record.request()->keepalive() && !fetch_record.request()->done())
                     in_flight_records.append(fetch_record);
             }
 
diff --git a/Libraries/LibWeb/Fetch/Infrastructure/FetchRecord.cpp b/Libraries/LibWeb/Fetch/Infrastructure/FetchRecord.cpp
index 6142ec9f2d0..8109690381a 100644
--- a/Libraries/LibWeb/Fetch/Infrastructure/FetchRecord.cpp
+++ b/Libraries/LibWeb/Fetch/Infrastructure/FetchRecord.cpp
@@ -5,6 +5,7 @@
  */
 
 #include <LibWeb/Fetch/Infrastructure/FetchRecord.h>
+#include <LibWeb/Fetch/Infrastructure/HTTP/Requests.h>
 
 namespace Web::Fetch::Infrastructure {
 
@@ -38,4 +39,10 @@ void FetchRecord::visit_edges(Visitor& visitor)
     visitor.visit(m_fetch_controller);
 }
 
+void FetchRecord::finalize()
+{
+    Base::finalize();
+    m_list_node.remove();
+}
+
 }
diff --git a/Libraries/LibWeb/Fetch/Infrastructure/FetchRecord.h b/Libraries/LibWeb/Fetch/Infrastructure/FetchRecord.h
index 758a87c37a3..14beca2a8d5 100644
--- a/Libraries/LibWeb/Fetch/Infrastructure/FetchRecord.h
+++ b/Libraries/LibWeb/Fetch/Infrastructure/FetchRecord.h
@@ -7,12 +7,11 @@
 #pragma once
 
 #include <LibWeb/Fetch/Infrastructure/FetchController.h>
-#include <LibWeb/Fetch/Infrastructure/HTTP/Requests.h>
 
 namespace Web::Fetch::Infrastructure {
 
 // https://fetch.spec.whatwg.org/#concept-fetch-record
-class FetchRecord : public JS::Cell {
+class FetchRecord final : public JS::Cell {
     GC_CELL(FetchRecord, JS::Cell);
     GC_DECLARE_ALLOCATOR(FetchRecord);
 
@@ -31,6 +30,7 @@ private:
     FetchRecord(GC::Ref<Infrastructure::Request>, GC::Ptr<FetchController>);
 
     virtual void visit_edges(Visitor&) override;
+    virtual void finalize() override;
 
     // https://fetch.spec.whatwg.org/#concept-request
     // A fetch record has an associated request (a request)
@@ -39,6 +39,11 @@ private:
     // https://fetch.spec.whatwg.org/#fetch-controller
     // A fetch record has an associated controller (a fetch controller or null)
     GC::Ptr<FetchController> m_fetch_controller { nullptr };
+
+    IntrusiveListNode<FetchRecord> m_list_node;
+
+public:
+    using List = IntrusiveList<&FetchRecord::m_list_node>;
 };
 
 }
diff --git a/Libraries/LibWeb/Fetch/Infrastructure/HTTP/Requests.h b/Libraries/LibWeb/Fetch/Infrastructure/HTTP/Requests.h
index b9504ca4f64..3dda56d1068 100644
--- a/Libraries/LibWeb/Fetch/Infrastructure/HTTP/Requests.h
+++ b/Libraries/LibWeb/Fetch/Infrastructure/HTTP/Requests.h
@@ -22,7 +22,6 @@
 #include <LibWeb/Fetch/Infrastructure/HTTP/Bodies.h>
 #include <LibWeb/Fetch/Infrastructure/HTTP/Headers.h>
 #include <LibWeb/HTML/PolicyContainers.h>
-#include <LibWeb/HTML/Scripting/Environments.h>
 
 namespace Web::Fetch::Infrastructure {
 
diff --git a/Libraries/LibWeb/HTML/Scripting/Environments.cpp b/Libraries/LibWeb/HTML/Scripting/Environments.cpp
index 69448ea35a6..e36488751ce 100644
--- a/Libraries/LibWeb/HTML/Scripting/Environments.cpp
+++ b/Libraries/LibWeb/HTML/Scripting/Environments.cpp
@@ -62,7 +62,6 @@ void EnvironmentSettingsObject::visit_edges(Cell::Visitor& visitor)
     visitor.visit(m_responsible_event_loop);
     visitor.visit(m_module_map);
     m_realm_execution_context->visit_edges(visitor);
-    visitor.visit(m_fetch_group);
     visitor.visit(m_storage_manager);
     visitor.visit(m_service_worker_registration_object_map);
     visitor.visit(m_service_worker_object_map);
diff --git a/Libraries/LibWeb/HTML/Scripting/Environments.h b/Libraries/LibWeb/HTML/Scripting/Environments.h
index 1a8dca0d4dd..a41a8c00ebb 100644
--- a/Libraries/LibWeb/HTML/Scripting/Environments.h
+++ b/Libraries/LibWeb/HTML/Scripting/Environments.h
@@ -11,6 +11,7 @@
 #include <LibJS/Forward.h>
 #include <LibURL/Origin.h>
 #include <LibURL/URL.h>
+#include <LibWeb/Fetch/Infrastructure/FetchRecord.h>
 #include <LibWeb/Forward.h>
 #include <LibWeb/HTML/EventLoop/EventLoop.h>
 #include <LibWeb/HTML/Scripting/ModuleMap.h>
@@ -116,7 +117,7 @@ public:
     EventLoop& responsible_event_loop();
 
     // https://fetch.spec.whatwg.org/#concept-fetch-group
-    Vector<GC::Ref<Fetch::Infrastructure::FetchRecord>>& fetch_group() { return m_fetch_group; }
+    auto& fetch_group() { return m_fetch_group; }
 
     SerializedEnvironmentSettingsObject serialize();
 
@@ -146,7 +147,7 @@ private:
 
     // https://fetch.spec.whatwg.org/#concept-fetch-record
     // A fetch group holds an ordered list of fetch records
-    Vector<GC::Ref<Fetch::Infrastructure::FetchRecord>> m_fetch_group;
+    Fetch::Infrastructure::FetchRecord::List m_fetch_group;
 
     // https://storage.spec.whatwg.org/#api
     // Each environment settings object has an associated StorageManager object.