From 417aadb5593663552777f30624ec5ee6bfacd514 Mon Sep 17 00:00:00 2001
From: Andreas Haas <[email protected]>
Date: Wed, 03 Jun 2026 00:39:24 -0700
Subject: [PATCH] [inspector] Turn V8Console and TaskInfo into an Oilpan objects

The TaskInfo object can thereby be attached directly to its
corresponding Task object. This guarantees that the TaskInfo object is
alive as long as it is referenced from the JS heap, even with sandbox
corruption.

As a side effect, this CL fixes a conflict in the assignment of external
pointer tags.

Bug: 513751951, 512591789
Change-Id: I7974751b1687bbf07c28b454e612ccfba7dbbdeb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7858052
Reviewed-by: Michael Lippautz <[email protected]>
Commit-Queue: Andreas Haas <[email protected]>
Cr-Commit-Position: refs/heads/main@{#107743}
---

diff --git a/include/v8-sandbox.h b/include/v8-sandbox.h
index b41a6ce2..bf44fa7 100644
--- a/include/v8-sandbox.h
+++ b/include/v8-sandbox.h
@@ -55,6 +55,12 @@
 
   kDefaultTag = 0x7000,
 
+  kFirstV8InternalTag = 0x7f00,
+  // V8-internal Oilpan objects that use v8::Object::Wrap() should go here.
+  kInspectorV8ConsoleTag,
+  kInspectorTaskInfoTag,
+  kLastV8InternalTag,
+
   kZappedEntryTag = 0x7ffd,
   kEvacuationEntryTag = 0x7ffe,
   kFreeEntryTag = 0x7fff,
@@ -62,6 +68,9 @@
   kLastTag = 0x7fff,
 };
 
+static_assert(static_cast<uint16_t>(CppHeapPointerTag::kLastV8InternalTag) <
+              static_cast<uint16_t>(CppHeapPointerTag::kZappedEntryTag));
+
 using CppHeapPointerTagRange = internal::TagRange<CppHeapPointerTag>;
 
 constexpr CppHeapPointerTagRange kAnyCppHeapPointer(
diff --git a/src/inspector/v8-console.cc b/src/inspector/v8-console.cc
index 2ca22d8..05d3bce 100644
--- a/src/inspector/v8-console.cc
+++ b/src/inspector/v8-console.cc
@@ -4,12 +4,17 @@
 
 #include "src/inspector/v8-console.h"
 
+#include <atomic>
+
+#include "include/cppgc/allocation.h"
 #include "include/v8-container.h"
 #include "include/v8-context.h"
+#include "include/v8-cppgc.h"
 #include "include/v8-function.h"
 #include "include/v8-inspector.h"
 #include "include/v8-microtask-queue.h"
 #include "include/v8-profiler.h"
+#include "include/v8-sandbox.h"
 #include "src/base/lazy-instance.h"
 #include "src/base/macros.h"
 #include "src/base/platform/time.h"
@@ -508,10 +513,10 @@
                                    ->NewInstance(isolate->GetCurrentContext())
                                    .ToLocalChecked();
 
-  auto taskInfo = std::make_unique<TaskInfo>(isolate, this, task);
+  auto* taskInfo = cppgc::MakeGarbageCollected<TaskInfo>(
+      isolate->GetCppHeap()->GetAllocationHandle(), isolate, this);
   void* taskId = taskInfo->Id();
-  auto [iter, inserted] = m_tasks.emplace(taskId, std::move(taskInfo));
-  CHECK(inserted);
+  v8::Object::Wrap<TaskInfo::kPointerTag>(isolate, task, taskInfo);
 
   String16 nameArgument = toProtocolString(isolate, info[0].As<v8::String>());
   StringView taskName =
@@ -521,13 +526,60 @@
   info.GetReturnValue().Set(task);
 }
 
-namespace {
-// This tag value has been picked arbitrarily between 0 and
-// V8_EXTERNAL_POINTER_TAG_COUNT.
-constexpr v8::ExternalPointerTypeTag kTaskInfoTag = 9;
-}  // namespace
+v8::Local<v8::Object> V8Console::wrapConsole(v8::Local<v8::Context> context) {
+  v8::Isolate* isolate = m_inspector->isolate();
+  if (!m_consoleWrapper.IsEmpty() && context == m_consoleContext) {
+    return m_consoleWrapper.Get(isolate);
+  }
+  if (m_consoleTemplate.IsEmpty()) {
+    v8::Local<v8::FunctionTemplate> consTemplate =
+        v8::FunctionTemplate::New(isolate);
+    m_consoleTemplate.Reset(isolate, consTemplate->InstanceTemplate());
+  }
+  v8::Local<v8::Object> consoleWrapper =
+      m_consoleTemplate.Get(isolate)->NewInstance(context).ToLocalChecked();
+  v8::Object::Wrap<kPointerTag>(isolate, consoleWrapper, this);
+  m_consoleWrapper.Reset(isolate, consoleWrapper);
+  m_consoleWrapper.SetWeak();
+  m_consoleContext.Reset(isolate, context);
+  m_consoleContext.SetWeak();
+  return consoleWrapper;
+}
 
-void V8Console::runTask(const v8::FunctionCallbackInfo<v8::Value>& info) {
+v8::Local<v8::ObjectTemplate> V8Console::taskTemplate() {
+  v8::Isolate* isolate = m_inspector->isolate();
+  if (!m_taskTemplate.IsEmpty()) {
+    return m_taskTemplate.Get(isolate);
+  }
+
+  v8::Local<v8::FunctionTemplate> consTemplate =
+      v8::FunctionTemplate::New(isolate);
+  v8::Local<v8::ObjectTemplate> taskTemplate = consTemplate->InstanceTemplate();
+  v8::Local<v8::FunctionTemplate> funcTemplate =
+      v8::FunctionTemplate::New(isolate, &TaskInfo::runTask);
+  taskTemplate->Set(isolate, "run", funcTemplate);
+
+  m_taskTemplate.Reset(isolate, taskTemplate);
+  return taskTemplate;
+}
+
+TaskInfo::TaskInfo(v8::Isolate* isolate, V8Console* console)
+    : m_isolate(isolate), m_console(console), m_id(new char(0)) {}
+
+TaskInfo::~TaskInfo() {
+  auto* inspector =
+      static_cast<V8InspectorImpl*>(v8::debug::GetInspector(m_isolate));
+  if (inspector) {
+    inspector->asyncTaskCanceled(Id());
+  }
+}
+
+void TaskInfo::Trace(cppgc::Visitor* visitor) const {
+  visitor->Trace(m_console);
+  v8::Object::Wrappable::Trace(visitor);
+}
+
+void TaskInfo::runTask(const v8::FunctionCallbackInfo<v8::Value>& info) {
   v8::Isolate* isolate = info.GetIsolate();
   if (info.Length() < 1 || !info[0]->IsFunction()) {
     isolate->ThrowError("First argument must be a function.");
@@ -536,23 +588,24 @@
   v8::Local<v8::Function> function = info[0].As<v8::Function>();
 
   v8::Local<v8::Object> task = info.This();
-  v8::Local<v8::Value> maybeTaskExternal;
-  if (!task->GetPrivate(isolate->GetCurrentContext(), taskInfoKey())
-           .ToLocal(&maybeTaskExternal)) {
-    // An exception is already thrown.
+  if (!task->IsApiWrapper()) {
+    isolate->ThrowError("'run' called with illegal receiver.");
     return;
   }
-
-  if (!maybeTaskExternal->IsExternal()) {
+  TaskInfo* taskInfo =
+      v8::Object::Unwrap<TaskInfo::kPointerTag, TaskInfo>(isolate, task);
+  if (!taskInfo) {
     isolate->ThrowError("'run' called with illegal receiver.");
     return;
   }
 
-  v8::Local<v8::External> taskExternal = maybeTaskExternal.As<v8::External>();
-  TaskInfo* taskInfo =
-      reinterpret_cast<TaskInfo*>(taskExternal->Value(kTaskInfoTag));
-
-  m_inspector->asyncTaskStarted(taskInfo->Id());
+  V8Console* console = taskInfo->m_console.Get();
+  CHECK_NOT_NULL(console);
+  if (!console->m_inspector) {
+    // Inspector has been cleared, so we're in shutdown.
+    return;
+  }
+  console->m_inspector->asyncTaskStarted(taskInfo->Id());
   {
 #ifdef V8_USE_PERFETTO
     TRACE_EVENT(TRACE_DISABLED_BY_DEFAULT("v8.inspector"), "V8Console::runTask",
@@ -572,56 +625,13 @@
       info.GetReturnValue().Set(result);
     }
   }
-  m_inspector->asyncTaskFinished(taskInfo->Id());
+  console->m_inspector->asyncTaskFinished(taskInfo->Id());
 }
 
-v8::Local<v8::Private> V8Console::taskInfoKey() {
-  v8::Isolate* isolate = m_inspector->isolate();
-  if (m_taskInfoKey.IsEmpty()) {
-    m_taskInfoKey.Reset(isolate, v8::Private::New(isolate));
-  }

diff --git a/test/unittests/inspector/inspector-unittest.cc b/test/unittests/inspector/inspector-unittest.cc
index 28fb48a..dccee46 100644
--- a/test/unittests/inspector/inspector-unittest.cc
+++ b/test/unittests/inspector/inspector-unittest.cc
@@ -284,45 +284,6 @@
       StringView(kCommand, sizeof(kCommand)));
 }
 
-TEST_F(InspectorTest, ApiCreatedTasksAreCleanedUp) {
-  v8::Isolate* isolate = v8_isolate();
-  v8::HandleScope handle_scope(isolate);
-
-  v8_inspector::V8InspectorClient default_client;
-  std::unique_ptr<v8_inspector::V8InspectorImpl> inspector =
-      std::make_unique<v8_inspector::V8InspectorImpl>(isolate, &default_client);
-  V8ContextInfo context_info(v8_context(), 1, toStringView(""));
-  inspector->contextCreated(context_info);
-
-  // Trigger V8Console creation.
-  v8_inspector::V8Console* console = inspector->console();
-  CHECK(console);
-
-  {
-    v8::HandleScope inner_handle_scope(isolate);
-    v8::MaybeLocal<v8::Value> result = TryRunJS(isolate, NewString(R"(
-      globalThis['task'] = console.createTask('Task');
-    )"));
-    CHECK(!result.IsEmpty());
-
-    // Run GC and check that the task is still here.
-    InvokeMajorGC();
-    CHECK_EQ(console->AllConsoleTasksForTest().size(), 1);
-  }
-
-  // Get rid of the task on the context, run GC and check we no longer have
-  // the TaskInfo in the inspector.
-  v8_context()->Global()->Delete(v8_context(), NewString("task")).Check();
-  {
-    // We need to invoke GC without stack, otherwise some objects may not be
-    // reclaimed because of conservative stack scanning.
-    DisableConservativeStackScanningScopeForTesting no_stack_scanning(
-        i_isolate()->heap());
-    InvokeMajorGC();
-  }
-  CHECK_EQ(console->AllConsoleTasksForTest().size(), 0);
-}
-
 TEST_F(InspectorTest, Evaluate) {
   v8::Isolate* isolate = v8_isolate();
   v8::HandleScope handle_scope(isolate);