From b6a5b7e18626ce0dee11a25bb0d51d2e84d3c057 Mon Sep 17 00:00:00 2001
From: Andreas Kling <andreas@ladybird.org>
Date: Tue, 12 Nov 2024 15:38:37 +0100
Subject: [PATCH] LibJS: Stop having AsyncFunctionDriverWrapper leak itself

Async functions whose promise is never resolved were leaking, since they
had a strong root JS::Handle on themselves.

This doesn't appear to actually be necessary, since the wrapper will be
kept alive as long as it's reachable (and if it's not reachable, nobody
is going to resolve/reject the promise either).

This fixes the vast majority of leaks on Speedometer, bringing memory
usage at the end of a full run from ~12 GiB to ~3 GiB.
---
 .../LibJS/Runtime/AsyncFunctionDriverWrapper.cpp      | 11 +----------
 Libraries/LibJS/Runtime/AsyncFunctionDriverWrapper.h  |  1 -
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/Libraries/LibJS/Runtime/AsyncFunctionDriverWrapper.cpp b/Libraries/LibJS/Runtime/AsyncFunctionDriverWrapper.cpp
index 7c02f509007..d5dd9ef1c9d 100644
--- a/Libraries/LibJS/Runtime/AsyncFunctionDriverWrapper.cpp
+++ b/Libraries/LibJS/Runtime/AsyncFunctionDriverWrapper.cpp
@@ -20,8 +20,7 @@ JS_DEFINE_ALLOCATOR(AsyncFunctionDriverWrapper);
 NonnullGCPtr<Promise> AsyncFunctionDriverWrapper::create(Realm& realm, GeneratorObject* generator_object)
 {
     auto top_level_promise = Promise::create(realm);
-    // Note: This generates a handle to itself, which it clears upon completing its execution
-    //       The top_level_promise is also kept alive by this Wrapper
+    // Note: The top_level_promise is also kept alive by this Wrapper
     auto wrapper = realm.heap().allocate<AsyncFunctionDriverWrapper>(realm, realm, *generator_object, *top_level_promise);
     // Prime the generator:
     // This runs until the first `await value;`
@@ -34,7 +33,6 @@ AsyncFunctionDriverWrapper::AsyncFunctionDriverWrapper(Realm& realm, NonnullGCPt
     : Promise(realm.intrinsics().promise_prototype())
     , m_generator_object(generator_object)
     , m_top_level_promise(top_level_promise)
-    , m_self_handle(make_handle(*this))
 {
 }
 
@@ -140,10 +138,6 @@ void AsyncFunctionDriverWrapper::continue_async_execution(VM& vm, Value value, b
             auto promise_value = TRY(result.get(vm, vm.names.value));
 
             if (TRY(result.get(vm, vm.names.done)).to_boolean()) {
-
-                // We should not execute anymore, so we are safe to allow ourselves to be GC'd.
-                m_self_handle = {};
-
                 // When returning a promise, we need to unwrap it.
                 if (promise_value.is_object() && is<Promise>(promise_value.as_object())) {
                     auto& returned_promise = static_cast<Promise&>(promise_value.as_object());
@@ -175,9 +169,6 @@ void AsyncFunctionDriverWrapper::continue_async_execution(VM& vm, Value value, b
 
     if (result.is_throw_completion()) {
         m_top_level_promise->reject(result.throw_completion().value().value_or(js_undefined()));
-
-        // We should not execute anymore, so we are safe to allow our selfs to be GC'd
-        m_self_handle = {};
     }
 
     // For the initial execution, the execution context will be popped for us later on by ECMAScriptFunctionObject.
diff --git a/Libraries/LibJS/Runtime/AsyncFunctionDriverWrapper.h b/Libraries/LibJS/Runtime/AsyncFunctionDriverWrapper.h
index 80c48672990..919f2d40a97 100644
--- a/Libraries/LibJS/Runtime/AsyncFunctionDriverWrapper.h
+++ b/Libraries/LibJS/Runtime/AsyncFunctionDriverWrapper.h
@@ -38,7 +38,6 @@ private:
     NonnullGCPtr<GeneratorObject> m_generator_object;
     NonnullGCPtr<Promise> m_top_level_promise;
     GCPtr<Promise> m_current_promise { nullptr };
-    Handle<AsyncFunctionDriverWrapper> m_self_handle;
     OwnPtr<ExecutionContext> m_suspended_execution_context;
 };