test: make IsolateData per-isolate in cctest

This ensures that we only create one IsolateData for each isolate
inthe cctest, since IsolateData are meant to be per-isolate.
We need to make the isolate and isolate_data static in the
test fixtures as a result, similar to how the event loops and
array buffer allocators are managed in the
NodeZeroIsolateTestFixture but it is fine because gtest ensures
that the Setup() and TearDown() of the fixtures are always run
in order and would never overlap in one process.

PR-URL: https://github.com/nodejs/node/pull/48450
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

test/cctest/node_test_fixture.cc

@@ -6,7 +6,8 @@ uv_loop_t NodeZeroIsolateTestFixture::current_loop;
 NodePlatformUniquePtr NodeZeroIsolateTestFixture::platform;
 TracingAgentUniquePtr NodeZeroIsolateTestFixture::tracing_agent;
 bool NodeZeroIsolateTestFixture::node_initialized = false;
-
+v8::Isolate* NodeTestFixture::isolate_ = nullptr;
+node::IsolateData* EnvironmentTestFixture::isolate_data_ = nullptr;
 
 void NodeTestEnvironment::SetUp() {
   NodeZeroIsolateTestFixture::tracing_agent =

test/cctest/node_test_fixture.h

@@ -67,6 +67,7 @@ class NodeTestEnvironment final : public ::testing::Environment {
   void TearDown() override;
 };
 
+class NodeTestFixture;
 
 class NodeZeroIsolateTestFixture : public ::testing::Test {
  protected:
@@ -104,12 +105,13 @@ class NodeZeroIsolateTestFixture : public ::testing::Test {
   }
 
   friend NodeTestEnvironment;
+  friend NodeTestFixture;
 };
 
 
 class NodeTestFixture : public NodeZeroIsolateTestFixture {
  protected:
-  v8::Isolate* isolate_;
+  static v8::Isolate* isolate_;
 
   void SetUp() override {
     NodeZeroIsolateTestFixture::SetUp();
@@ -130,7 +132,22 @@ class NodeTestFixture : public NodeZeroIsolateTestFixture {
 
 
 class EnvironmentTestFixture : public NodeTestFixture {
- public:
+ protected:
+  static node::IsolateData* isolate_data_;
+
+  void SetUp() override {
+    NodeTestFixture::SetUp();
+    isolate_data_ = node::CreateIsolateData(NodeTestFixture::isolate_,
+                                            &NodeTestFixture::current_loop,
+                                            platform.get());
+    CHECK_NE(nullptr, isolate_data_);
+  }
+
+  void TearDown() override {
+    node::FreeIsolateData(isolate_data_);
+    NodeTestFixture::TearDown();
+  }
+
   class Env {
    public:
     Env(const v8::HandleScope& handle_scope,
@@ -142,23 +159,20 @@ class EnvironmentTestFixture : public NodeTestFixture {
       CHECK(!context_.IsEmpty());
       context_->Enter();
 
-      isolate_data_ = node::CreateIsolateData(isolate,
-                                              &NodeTestFixture::current_loop,
-                                              platform.get());
-      CHECK_NE(nullptr, isolate_data_);
       std::vector<std::string> args(*argv, *argv + 1);
       std::vector<std::string> exec_args(*argv, *argv + 1);
-      environment_ = node::CreateEnvironment(isolate_data_,
-                                             context_,
-                                             args,
-                                             exec_args,
-                                             flags);
+      DCHECK_EQ(EnvironmentTestFixture::isolate_data_->isolate(), isolate);
+      environment_ =
+          node::CreateEnvironment(EnvironmentTestFixture::isolate_data_,
+                                  context_,
+                                  args,
+                                  exec_args,
+                                  flags);
       CHECK_NE(nullptr, environment_);
     }
 
     ~Env() {
       node::FreeEnvironment(environment_);
-      node::FreeIsolateData(isolate_data_);
       context_->Exit();
     }
 
@@ -175,7 +189,6 @@ class EnvironmentTestFixture : public NodeTestFixture {
 
    private:
     v8::Local<v8::Context> context_;
-    node::IsolateData* isolate_data_;
     node::Environment* environment_;
   };
 };

test/cctest/test_environment.cc

@@ -30,7 +30,7 @@ static std::string cb_1_arg;  // NOLINT(runtime/string)
 class EnvironmentTest : public EnvironmentTestFixture {
  private:
   void TearDown() override {
-    NodeTestFixture::TearDown();
+    EnvironmentTestFixture::TearDown();
     called_cb_1 = false;
     called_cb_2 = false;
     called_cb_ordered_1 = false;
@@ -674,12 +674,8 @@ TEST_F(EnvironmentTest, NestedMicrotaskQueue) {
       v8::String::NewFromUtf8Literal(isolate_, "mustCall"),
       must_call).Check();
 
-  node::IsolateData* isolate_data = node::CreateIsolateData(
-      isolate_, &NodeTestFixture::current_loop, platform.get());
-  CHECK_NE(nullptr, isolate_data);
-
-  node::Environment* env = node::CreateEnvironment(
-      isolate_data, context, {}, {});
+  node::Environment* env =
+      node::CreateEnvironment(isolate_data_, context, {}, {});
   CHECK_NE(nullptr, env);
 
   v8::Local<v8::Function> eval_in_env = node::LoadEnvironment(
@@ -718,7 +714,6 @@ TEST_F(EnvironmentTest, NestedMicrotaskQueue) {
   EXPECT_EQ(callback_calls, (IntVec { 1, 3, 6, 2, 4, 7, 5 }));
 
   node::FreeEnvironment(env);
-  node::FreeIsolateData(isolate_data);
 }
 
 static bool interrupted = false;
@@ -733,19 +728,15 @@ TEST_F(EnvironmentTest, RequestInterruptAtExit) {
   CHECK(!context.IsEmpty());
   context->Enter();
 
-  node::IsolateData* isolate_data = node::CreateIsolateData(
-      isolate_, &NodeTestFixture::current_loop, platform.get());
-  CHECK_NE(nullptr, isolate_data);
   std::vector<std::string> args(*argv, *argv + 1);
   std::vector<std::string> exec_args(*argv, *argv + 1);
   node::Environment* environment =
-      node::CreateEnvironment(isolate_data, context, args, exec_args);
+      node::CreateEnvironment(isolate_data_, context, args, exec_args);
   CHECK_NE(nullptr, environment);
 
   node::RequestInterrupt(environment, OnInterrupt, nullptr);
   node::FreeEnvironment(environment);
   EXPECT_TRUE(interrupted);
 
-  node::FreeIsolateData(isolate_data);
   context->Exit();
 }

test/cctest/test_node_api.cc

@@ -16,7 +16,7 @@ class NodeApiTest : public EnvironmentTestFixture {
  private:
   void SetUp() override { EnvironmentTestFixture::SetUp(); }
 
-  void TearDown() override { NodeTestFixture::TearDown(); }
+  void TearDown() override { EnvironmentTestFixture::TearDown(); }
 };
 
 TEST_F(NodeApiTest, CreateNodeApiEnv) {

test/cctest/test_report.cc

@@ -20,7 +20,7 @@ bool report_callback_called = false;
 class ReportTest : public EnvironmentTestFixture {
  private:
   void TearDown() override {
-    NodeTestFixture::TearDown();
+    EnvironmentTestFixture::TearDown();
     report_callback_called = false;
   }
 };