[inspector] Dispose the isolate in the inspector tests

At the moment the inspector tests do not dispose the isolate. This is a
problem because the disposal of the isolate is used to stop the
execution of background tasks. The missing disposal of the isolate
caused flaky tests on the bots recently. With this CL the isolates of
the inspector tests get disposed.

The disposal of the isolate requires the following changes: 1) Store the
isolate in a unique_ptr so that it gets disposed when the isolate-data
gets disposed. It is necessary to use the unique_ptr so that the isolate
gets disposed after other members of isolate-data get disposed.  2)
Dispose all sessions. The reason is that the sessions require the
isolate to exist when they get disposed because they own handles.
Sessions, however, are stored in a static map, whereas the isolate is
stored indirectly in a local variable of the main function. Since local
variables get disposed before the static map is cleared, we have to
clear the map before the end of the main function.

R=kozyatinskiy@chromium.org

Change-Id: Icb33184de254638b6cdfb899e940f18e6064cd69
Reviewed-on: https://chromium-review.googlesource.com/774885Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49424}

test/inspector/inspector-test.cc

@@ -322,6 +322,8 @@ class UtilsExtension : public IsolateData::SetupGlobalTask {
     backend_runner_ = runner;
   }
 
+  static void ClearAllSessions() { channels_.clear(); }
+
  private:
   static TaskRunner* backend_runner_;
 
@@ -931,5 +933,6 @@ int main(int argc, char* argv[]) {
   backend_runner.Join();
 
   delete startup_data.data;
+  UtilsExtension::ClearAllSessions();
   return 0;
 }

test/inspector/isolate-data.cc

@@ -63,19 +63,20 @@ IsolateData::IsolateData(TaskRunner* task_runner,
   params.array_buffer_allocator =
       v8::ArrayBuffer::Allocator::NewDefaultAllocator();
   params.snapshot_blob = startup_data;
-  isolate_ = v8::Isolate::New(params);
+  isolate_.reset(v8::Isolate::New(params));
   isolate_->SetMicrotasksPolicy(v8::MicrotasksPolicy::kScoped);
   if (with_inspector) {
     isolate_->AddMessageListener(&IsolateData::MessageHandler);
     isolate_->SetPromiseRejectCallback(&IsolateData::PromiseRejectHandler);
-    inspector_ = v8_inspector::V8Inspector::create(isolate_, this);
+    inspector_ = v8_inspector::V8Inspector::create(isolate_.get(), this);
   }
-  v8::HandleScope handle_scope(isolate_);
+  v8::HandleScope handle_scope(isolate_.get());
   not_inspectable_private_.Reset(
-      isolate_, v8::Private::ForApi(isolate_, v8::String::NewFromUtf8(
-                                                  isolate_, "notInspectable",
-                                                  v8::NewStringType::kNormal)
-                                                  .ToLocalChecked()));
+      isolate_.get(),
+      v8::Private::ForApi(isolate_.get(), v8::String::NewFromUtf8(
+                                              isolate_.get(), "notInspectable",
+                                              v8::NewStringType::kNormal)
+                                              .ToLocalChecked()));
 }
 
 IsolateData* IsolateData::FromContext(v8::Local<v8::Context> context) {
@@ -84,27 +85,27 @@ IsolateData* IsolateData::FromContext(v8::Local<v8::Context> context) {
 }
 
 int IsolateData::CreateContextGroup() {
-  v8::HandleScope handle_scope(isolate_);
+  v8::HandleScope handle_scope(isolate_.get());
   v8::Local<v8::ObjectTemplate> global_template =
-      v8::ObjectTemplate::New(isolate_);
+      v8::ObjectTemplate::New(isolate_.get());
   for (auto it = setup_global_tasks_.begin(); it != setup_global_tasks_.end();
        ++it) {
-    (*it)->Run(isolate_, global_template);
+    (*it)->Run(isolate_.get(), global_template);
   }
   v8::Local<v8::Context> context =
-      v8::Context::New(isolate_, nullptr, global_template);
+      v8::Context::New(isolate_.get(), nullptr, global_template);
   context->SetAlignedPointerInEmbedderData(kIsolateDataIndex, this);
   int context_group_id = ++last_context_group_id_;
   // Should be 2-byte aligned.
   context->SetAlignedPointerInEmbedderData(
       kContextGroupIdIndex, reinterpret_cast<void*>(context_group_id * 2));
-  contexts_[context_group_id].Reset(isolate_, context);
+  contexts_[context_group_id].Reset(isolate_.get(), context);
   if (inspector_) FireContextCreated(context, context_group_id);
   return context_group_id;
 }
 
 v8::Local<v8::Context> IsolateData::GetContext(int context_group_id) {
-  return contexts_[context_group_id].Get(isolate_);
+  return contexts_[context_group_id].Get(isolate_.get());
 }
 
 int IsolateData::GetContextGroupId(v8::Local<v8::Context> context) {
@@ -126,7 +127,7 @@ void IsolateData::RegisterModule(v8::Local<v8::Context> context,
   }
   v8::Local<v8::Value> result;
   if (!module->Evaluate(context).ToLocal(&result)) return;
-  modules_[name] = v8::Global<v8::Module>(isolate_, module);
+  modules_[name] = v8::Global<v8::Module>(isolate_.get(), module);
 }
 
 // static
@@ -134,8 +135,8 @@ v8::MaybeLocal<v8::Module> IsolateData::ModuleResolveCallback(
     v8::Local<v8::Context> context, v8::Local<v8::String> specifier,
     v8::Local<v8::Module> referrer) {
   IsolateData* data = IsolateData::FromContext(context);
-  std::string str = *v8::String::Utf8Value(data->isolate_, specifier);
-  return data->modules_[ToVector(specifier)].Get(data->isolate_);
+  std::string str = *v8::String::Utf8Value(data->isolate(), specifier);
+  return data->modules_[ToVector(specifier)].Get(data->isolate());
 }
 
 int IsolateData::ConnectSession(int context_group_id,
@@ -206,7 +207,8 @@ void IsolateData::AddInspectedObject(int session_id,
                                      v8::Local<v8::Value> object) {
   auto it = sessions_.find(session_id);
   if (it == sessions_.end()) return;
-  std::unique_ptr<Inspectable> inspectable(new Inspectable(isolate_, object));
+  std::unique_ptr<Inspectable> inspectable(
+      new Inspectable(isolate_.get(), object));
   it->second->addInspectedObject(std::move(inspectable));
 }
 
@@ -363,7 +365,7 @@ double IsolateData::currentTimeMS() {
 }
 
 void IsolateData::SetMemoryInfo(v8::Local<v8::Value> memory_info) {
-  memory_info_.Reset(isolate_, memory_info);
+  memory_info_.Reset(isolate_.get(), memory_info);
 }
 
 void IsolateData::SetLogConsoleApiMessageCalls(bool log) {
@@ -393,11 +395,11 @@ void IsolateData::consoleAPIMessage(int contextGroupId,
                                     unsigned lineNumber, unsigned columnNumber,
                                     v8_inspector::V8StackTrace* stack) {
   if (!log_console_api_message_calls_) return;
-  Print(isolate_, message);
+  Print(isolate_.get(), message);
   fprintf(stdout, " (");
-  Print(isolate_, url);
+  Print(isolate_.get(), url);
   fprintf(stdout, ":%d:%d)", lineNumber, columnNumber);
-  Print(isolate_, stack->toString()->string());
+  Print(isolate_.get(), stack->toString()->string());
   fprintf(stdout, "\n");
 }
 

test/inspector/isolate-data.h

@@ -30,7 +30,7 @@ class IsolateData : public v8_inspector::V8InspectorClient {
               v8::StartupData* startup_data, bool with_inspector);
   static IsolateData* FromContext(v8::Local<v8::Context> context);
 
-  v8::Isolate* isolate() const { return isolate_; }
+  v8::Isolate* isolate() const { return isolate_.get(); }
   TaskRunner* task_runner() const { return task_runner_; }
 
   // Setting things up.
@@ -109,9 +109,17 @@ class IsolateData : public v8_inspector::V8InspectorClient {
   bool isInspectableHeapObject(v8::Local<v8::Object>) override;
   void maxAsyncCallStackDepthChanged(int depth) override;
 
+  // The isolate gets deleted by its {Dispose} method, not by the default
+  // deleter. Therefore we have to define a custom deleter for the unique_ptr to
+  // call {Dispose}. We have to use the unique_ptr so that the isolate get
+  // disposed in the right order, relative to other member variables.
+  struct IsolateDeleter {
+    void operator()(v8::Isolate* isolate) const { isolate->Dispose(); }
+  };
+
   TaskRunner* task_runner_;
   SetupGlobalTasks setup_global_tasks_;
-  v8::Isolate* isolate_;
+  std::unique_ptr<v8::Isolate, IsolateDeleter> isolate_;
   std::unique_ptr<v8_inspector::V8Inspector> inspector_;
   int last_context_group_id_ = 0;
   std::map<int, v8::Global<v8::Context>> contexts_;