[d8] Make the profileEnd callback isolate-specific

The OnProfileEndListener callback has to be reset before the isolate
dies to avoid a use-after-free when the Global which holds the callback
gets released.

Drive-by change: make the OnProfileEndListener callback
isolate-specific. At the moment a `profileEnd` call in IsolateA could
trigger the OnProfileEndListener callback of IsolateB, which could
cause all kinds of data races (the callback would access the isolate,
but the isolate is not supposed to get accessed by multiple threads
concurrently. With this CL there is one callback per isolate.

R=clemensb@chromium.org

Bug: chromium:1395237
Change-Id: Ifaa5b883a231f5519a3bfeb6187fb7d8faa02b02
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4076465
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84646}

src/d8/d8-console.cc

@@ -136,7 +136,7 @@ void D8Console::ProfileEnd(const debug::ConsoleCallArguments& args,
   if (!profiler_) return;
   CpuProfile* profile = profiler_->StopProfiling(String::Empty(isolate_));
   profiler_active_ = false;
-  if (Shell::HasOnProfileEndListener()) {
+  if (Shell::HasOnProfileEndListener(isolate_)) {
     StringOutputStream out;
     profile->Serialize(&out);
     Shell::TriggerOnProfileEndListener(isolate_, out.result());

src/d8/d8.cc

@@ -465,8 +465,8 @@ CounterCollection* Shell::counters_ = &local_counters_;
 base::LazyMutex Shell::context_mutex_;
 const base::TimeTicks Shell::kInitialTicks = base::TimeTicks::Now();
 Global<Function> Shell::stringify_function_;
-Global<Function> Shell::profile_end_callback_;
-Global<Context> Shell::profile_end_callback_context_;
+std::map<Isolate*, std::pair<Global<Function>, Global<Context>>>
+    Shell::profiler_end_callback_;
 base::LazyMutex Shell::workers_mutex_;
 bool Shell::allow_new_workers_ = true;
 std::unordered_set<std::shared_ptr<Worker>> Shell::running_workers_;
@@ -2519,14 +2519,17 @@ void Shell::ProfilerSetOnProfileEndListener(
     isolate->ThrowError("The OnProfileEnd listener has to be a function");
     return;
   }
-  profile_end_callback_.Reset(isolate, args[0].As<Function>());
-  profile_end_callback_context_.Reset(isolate, isolate->GetCurrentContext());
+  profiler_end_callback_[isolate] =
+      std::make_pair(Global<Function>(isolate, args[0].As<Function>()),
+                     Global<Context>(isolate, isolate->GetCurrentContext()));
 }
 
-bool Shell::HasOnProfileEndListener() {
-  CHECK_EQ(profile_end_callback_.IsEmpty(),
-           profile_end_callback_context_.IsEmpty());
-  return !profile_end_callback_.IsEmpty();
+bool Shell::HasOnProfileEndListener(Isolate* isolate) {
+  return profiler_end_callback_.find(isolate) != profiler_end_callback_.end();
+}
+
+void Shell::ResetOnProfileEndListener(Isolate* isolate) {
+  profiler_end_callback_.erase(isolate);
 }
 
 void Shell::ProfilerTriggerSample(
@@ -2539,19 +2542,15 @@ void Shell::ProfilerTriggerSample(
 }
 
 void Shell::TriggerOnProfileEndListener(Isolate* isolate, std::string profile) {
-  CHECK(HasOnProfileEndListener());
+  CHECK(HasOnProfileEndListener(isolate));
   Local<Value> argv[1] = {
       String::NewFromUtf8(isolate, profile.c_str()).ToLocalChecked()};
-  Local<Context> context = profile_end_callback_context_.Get(isolate);
+  auto& callback_pair = profiler_end_callback_[isolate];
+  Local<Function> callback = callback_pair.first.Get(isolate);
+  Local<Context> context = callback_pair.second.Get(isolate);
   TryCatch try_catch(isolate);
   try_catch.SetVerbose(true);
-  USE(profile_end_callback_.Get(isolate)->Call(context, Undefined(isolate), 1,
-                                               argv));
-  // The profiler callback may have been set up on a worker. We reset the
-  // callbacks now to avoid problems in the shutdown sequence of the worker and
-  // the main thread.
-  profile_end_callback_.Reset();
-  profile_end_callback_context_.Reset();
+  USE(callback->Call(context, Undefined(isolate), 1, argv));
 }
 
 void WriteToFile(FILE* file, const v8::FunctionCallbackInfo<v8::Value>& args) {
@@ -3583,7 +3582,7 @@ Local<ObjectTemplate> Shell::CreateD8Template(Isolate* isolate) {
   {
     Local<ObjectTemplate> profiler_template = ObjectTemplate::New(isolate);
     profiler_template->Set(
-        isolate, "setOneShotOnProfileEndListener",
+        isolate, "setOnProfileEndListener",
         FunctionTemplate::New(isolate, ProfilerSetOnProfileEndListener));
     profiler_template->Set(
         isolate, "triggerSample",
@@ -4482,6 +4481,7 @@ void SourceGroup::ExecuteInThread() {
     done_semaphore_.Signal();
   }
 
+  Shell::ResetOnProfileEndListener(isolate);
   isolate->Dispose();
 }
 
@@ -4766,6 +4766,7 @@ void Worker::ExecuteInThread() {
     task_manager_ = nullptr;
   }
 
+  Shell::ResetOnProfileEndListener(isolate_);
   context_.Reset();
   platform::NotifyIsolateShutdown(g_default_platform, isolate_);
   isolate_->Dispose();
@@ -5926,6 +5927,7 @@ int Shell::Main(int argc, char* argv[]) {
 
             result = RunMain(isolate2, false);
           }
+          ResetOnProfileEndListener(isolate2);
           isolate2->Dispose();
         }
 
@@ -5974,8 +5976,7 @@ int Shell::Main(int argc, char* argv[]) {
       cached_code_map_.clear();
       evaluation_context_.Reset();
       stringify_function_.Reset();
-      profile_end_callback_.Reset();
-      profile_end_callback_context_.Reset();
+      ResetOnProfileEndListener(isolate);
       CollectGarbage(isolate);
 #ifdef V8_FUZZILLI
       // Send result to parent (fuzzilli) and reset edge guards.

src/d8/d8.h

@@ -603,11 +603,13 @@ class Shell : public i::AllStatic {
   static void ProfilerTriggerSample(
       const v8::FunctionCallbackInfo<v8::Value>& args);
 
-  static bool HasOnProfileEndListener();
+  static bool HasOnProfileEndListener(Isolate* isolate);
 
   static void TriggerOnProfileEndListener(Isolate* isolate,
                                           std::string profile);
 
+  static void ResetOnProfileEndListener(Isolate* isolate);
+
   static void Print(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void PrintErr(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void WriteStdout(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -745,8 +747,8 @@ class Shell : public i::AllStatic {
   static base::OnceType quit_once_;
   static Global<Function> stringify_function_;
 
-  static Global<Function> profile_end_callback_;
-  static Global<Context> profile_end_callback_context_;
+  static std::map<Isolate*, std::pair<Global<Function>, Global<Context>>>
+      profiler_end_callback_;
 
   static const char* stringify_source_;
   static CounterMap* counter_map_;

test/mjsunit/wasm/log-code-after-post-message.js

@@ -17,7 +17,7 @@ function workerCode() {
         });
   };
 
-  d8.profiler.setOneShotOnProfileEndListener(WorkerOnProfileEnd);
+  d8.profiler.setOnProfileEndListener(WorkerOnProfileEnd);
   // Code logging happens for all code objects when profiling gets started,
   // and when new code objects appear after profiling has started. We want to
   // test the second scenario here. As new code objects appear as the