From aae45ca822573789387f15195ef1276cdfcbdd41 Mon Sep 17 00:00:00 2001
From: Seth Brenith <seth.brenith@microsoft.com>
Date: Tue, 8 Feb 2022 10:58:41 -0800
Subject: [PATCH] Avoid leaking Promises when detaching debugger

When the debugger is active and a Promise begins executing,
Isolate::PushPromise adds a global handle for that Promise. If the
debugger is no longer attached when the Promise finishes executing, then
there is no corresponding call to PopPromise which would clean up the
global handle. To avoid leaking memory in that case, we should clean up
the Promise stack when detaching the debugger.

Bug: v8:12613
Change-Id: I47a2c37713b43b482e23e2457e96fba5f52623f4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3448949
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#79017}
---
 src/debug/debug.cc       | 6 ++++++
 src/debug/debug.h        | 2 ++
 src/execution/isolate.cc | 5 +++--
 src/execution/isolate.h  | 2 +-
 4 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/debug/debug.cc b/src/debug/debug.cc
index 72d92da27e..c4caca3323 100644
--- a/src/debug/debug.cc
+++ b/src/debug/debug.cc
@@ -440,6 +440,7 @@ void Debug::Unload() {
   ClearStepping();
   RemoveAllCoverageInfos();
   ClearAllDebuggerHints();
+  ClearGlobalPromiseStack();
   debug_delegate_ = nullptr;
 }
 
@@ -1990,6 +1991,11 @@ void Debug::FreeDebugInfoListNode(DebugInfoListNode* prev,
   delete node;
 }
 
+void Debug::ClearGlobalPromiseStack() {
+  while (isolate_->PopPromise()) {
+  }
+}
+
 bool Debug::IsBreakAtReturn(JavaScriptFrame* frame) {
   RCS_SCOPE(isolate_, RuntimeCallCounterId::kDebugger);
   HandleScope scope(isolate_);
diff --git a/src/debug/debug.h b/src/debug/debug.h
index eba382258e..71fc6ea015 100644
--- a/src/debug/debug.h
+++ b/src/debug/debug.h
@@ -475,6 +475,8 @@ class V8_EXPORT_PRIVATE Debug {
                      DebugInfoListNode** curr);
   void FreeDebugInfoListNode(DebugInfoListNode* prev, DebugInfoListNode* node);
 
+  void ClearGlobalPromiseStack();
+
   void SetTemporaryObjectTrackingDisabled(bool disabled);
   bool GetTemporaryObjectTrackingDisabled() const;
 
diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc
index aa2b637a2b..4f3e15f4b5 100644
--- a/src/execution/isolate.cc
+++ b/src/execution/isolate.cc
@@ -2599,14 +2599,15 @@ void Isolate::PushPromise(Handle<JSObject> promise) {
   tltop->promise_on_stack_ = new PromiseOnStack(global_promise, prev);
 }
 
-void Isolate::PopPromise() {
+bool Isolate::PopPromise() {
   ThreadLocalTop* tltop = thread_local_top();
-  if (tltop->promise_on_stack_ == nullptr) return;
+  if (tltop->promise_on_stack_ == nullptr) return false;
   PromiseOnStack* prev = tltop->promise_on_stack_->prev();
   Handle<Object> global_promise = tltop->promise_on_stack_->promise();
   delete tltop->promise_on_stack_;
   tltop->promise_on_stack_ = prev;
   global_handles()->Destroy(global_promise.location());
+  return true;
 }
 
 namespace {
diff --git a/src/execution/isolate.h b/src/execution/isolate.h
index 605bd17384..019112f06c 100644
--- a/src/execution/isolate.h
+++ b/src/execution/isolate.h
@@ -855,7 +855,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
 
   // Push and pop a promise and the current try-catch handler.
   void PushPromise(Handle<JSObject> promise);
-  void PopPromise();
+  bool PopPromise();
 
   // Return the relevant Promise that a throw/rejection pertains to, based
   // on the contents of the Promise stack