Browse Source

LibWeb: Avoid introducing a reference cycle in ResourceLoader::load()

Previously we were kinda sorta resolving the reference cycle, but let's
just keep the requests in a hashtable instead of relying on hard to
track refcount tricks.
Fixes #7314.
Ali Mohammad Pur 3 years ago
parent
commit
e780ee2832

+ 5 - 5
Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp

@@ -191,7 +191,8 @@ void ResourceLoader::load(LoadRequest& request, Function<void(ReadonlyBytes, con
                 error_callback(start_request_failure_msg, {});
                 error_callback(start_request_failure_msg, {});
             return;
             return;
         }
         }
-        protocol_request->on_buffered_request_finish = [this, success_callback = move(success_callback), error_callback = move(error_callback), log_success, log_failure, request, protocol_request](bool success, auto, auto& response_headers, auto status_code, ReadonlyBytes payload) {
+        m_active_requests.set(*protocol_request);
+        protocol_request->on_buffered_request_finish = [this, success_callback = move(success_callback), error_callback = move(error_callback), log_success, log_failure, request, &protocol_request = *protocol_request](bool success, auto, auto& response_headers, auto status_code, ReadonlyBytes payload) {
             --m_pending_loads;
             --m_pending_loads;
             if (on_load_counter_change)
             if (on_load_counter_change)
                 on_load_counter_change();
                 on_load_counter_change();
@@ -202,12 +203,11 @@ void ResourceLoader::load(LoadRequest& request, Function<void(ReadonlyBytes, con
                     error_callback(http_load_failure_msg, {});
                     error_callback(http_load_failure_msg, {});
                 return;
                 return;
             }
             }
-            deferred_invoke([protocol_request] {
-                // Clear circular reference of `protocol_request` captured by copy
-                const_cast<Protocol::Request&>(*protocol_request).on_buffered_request_finish = nullptr;
-            });
             success_callback(payload, response_headers, status_code);
             success_callback(payload, response_headers, status_code);
             log_success(request);
             log_success(request);
+            deferred_invoke([this, &protocol_request] {
+                m_active_requests.remove(protocol_request);
+            });
         };
         };
         protocol_request->set_should_buffer_all_input(true);
         protocol_request->set_should_buffer_all_input(true);
         protocol_request->on_certificate_requested = []() -> Protocol::Request::CertificateAndKey {
         protocol_request->on_certificate_requested = []() -> Protocol::Request::CertificateAndKey {

+ 2 - 0
Userland/Libraries/LibWeb/Loader/ResourceLoader.h

@@ -13,6 +13,7 @@
 
 
 namespace Protocol {
 namespace Protocol {
 class RequestClient;
 class RequestClient;
+class Request;
 }
 }
 
 
 namespace Web {
 namespace Web {
@@ -53,6 +54,7 @@ private:
 
 
     int m_pending_loads { 0 };
     int m_pending_loads { 0 };
 
 
+    HashTable<NonnullRefPtr<Protocol::Request>> m_active_requests;
     RefPtr<Protocol::RequestClient> m_protocol_client;
     RefPtr<Protocol::RequestClient> m_protocol_client;
     String m_user_agent;
     String m_user_agent;
 };
 };