src: refactor bookkeeping of bootstrap status

src/env-inl.h

@@ -780,8 +780,12 @@ inline bool Environment::has_run_bootstrapping_code() const {
   return has_run_bootstrapping_code_;
 }
 
-inline void Environment::set_has_run_bootstrapping_code(bool value) {
-  has_run_bootstrapping_code_ = value;
+inline void Environment::DoneBootstrapping() {
+  has_run_bootstrapping_code_ = true;
+  // This adjusts the return value of base_object_created_after_bootstrap() so
+  // that tests that check the count do not have to account for internally
+  // created BaseObjects.
+  base_object_created_by_bootstrap_ = base_object_count_;
 }
 
 inline bool Environment::has_serialized_options() const {
@@ -1085,8 +1089,12 @@ void Environment::modify_base_object_count(int64_t delta) {
   base_object_count_ += delta;
 }
 
+int64_t Environment::base_object_created_after_bootstrap() const {
+  return base_object_count_ - base_object_created_by_bootstrap_;
+}
+
 int64_t Environment::base_object_count() const {
-  return base_object_count_ - initial_base_object_count_;
+  return base_object_count_;
 }
 
 void Environment::set_main_utf16(std::unique_ptr<v8::String::Value> str) {

src/env.cc

@@ -414,10 +414,6 @@ Environment::Environment(IsolateData* isolate_data,
                                       "args",
                                       std::move(traced_value));
   }
-
-  // This adjusts the return value of base_object_count() so that tests that
-  // check the count do not have to account for internally created BaseObjects.
-  initial_base_object_count_ = base_object_count();
 }
 
 Environment::Environment(IsolateData* isolate_data,
@@ -460,10 +456,6 @@ void Environment::InitializeMainContext(Local<Context> context,
                            per_process::node_start_time);
   performance_state_->Mark(performance::NODE_PERFORMANCE_MILESTONE_V8_START,
                            performance::performance_v8_start);
-
-  // This adjusts the return value of base_object_count() so that tests that
-  // check the count do not have to account for internally created BaseObjects.
-  initial_base_object_count_ = base_object_count();
 }
 
 Environment::~Environment() {
@@ -654,7 +646,6 @@ void Environment::RunCleanup() {
   TraceEventScope trace_scope(TRACING_CATEGORY_NODE1(environment),
                               "RunCleanup", this);
   bindings_.clear();
-  initial_base_object_count_ = 0;
   CleanupHandles();
 
   while (!cleanup_hooks_.empty() ||

src/env.h

@@ -1168,7 +1168,7 @@ class Environment : public MemoryRetainer {
   inline void add_refs(int64_t diff);
 
   inline bool has_run_bootstrapping_code() const;
-  inline void set_has_run_bootstrapping_code(bool has_run_bootstrapping_code);
+  inline void DoneBootstrapping();
 
   inline bool has_serialized_options() const;
   inline void set_has_serialized_options(bool has_serialized_options);
@@ -1354,6 +1354,7 @@ class Environment : public MemoryRetainer {
   // no memory leaks caused by BaseObjects staying alive longer than expected
   // (in particular, no circular BaseObjectPtr references).
   inline void modify_base_object_count(int64_t delta);
+  inline int64_t base_object_created_after_bootstrap() const;
   inline int64_t base_object_count() const;
 
   inline int32_t stack_trace_limit() const { return 10; }
@@ -1547,7 +1548,7 @@ class Environment : public MemoryRetainer {
   bool started_cleanup_ = false;
 
   int64_t base_object_count_ = 0;
-  int64_t initial_base_object_count_ = 0;
+  int64_t base_object_created_by_bootstrap_ = 0;
   std::atomic_bool is_stopping_ { false };
 
   std::unordered_set<int> unmanaged_fds_;

src/node.cc

@@ -415,7 +415,7 @@ MaybeLocal<Value> Environment::RunBootstrapping() {
   CHECK(req_wrap_queue()->IsEmpty());
   CHECK(handle_wrap_queue()->IsEmpty());
 
-  set_has_run_bootstrapping_code(true);
+  DoneBootstrapping();
 
   return scope.Escape(result);
 }

src/node_main_instance.cc

@@ -209,10 +209,18 @@ NodeMainInstance::CreateMainEnvironment(int* exit_code,
                                     {DeserializeNodeInternalFields, env.get()})
                   .ToLocalChecked();
 
+    CHECK(!context.IsEmpty());
+    Context::Scope context_scope(context);
     InitializeContextRuntime(context);
     SetIsolateErrorHandlers(isolate_, {});
+    env->InitializeMainContext(context, env_info);
+#if HAVE_INSPECTOR
+    env->InitializeInspector({});
+#endif
+    env->DoneBootstrapping();
   } else {
     context = NewContext(isolate_);
+    CHECK(!context.IsEmpty());
     Context::Scope context_scope(context);
     env.reset(new Environment(isolate_data_.get(),
                               context,
@@ -221,24 +229,16 @@ NodeMainInstance::CreateMainEnvironment(int* exit_code,
                               nullptr,
                               EnvironmentFlags::kDefaultFlags,
                               {}));
-  }
-
-  CHECK(!context.IsEmpty());
-  Context::Scope context_scope(context);
-
-  env->InitializeMainContext(context, env_info);
-
 #if HAVE_INSPECTOR
-  env->InitializeInspector({});
+    env->InitializeInspector({});
 #endif
-
-  if (!deserialize_mode_ && env->RunBootstrapping().IsEmpty()) {
-    return nullptr;
+    if (env->RunBootstrapping().IsEmpty()) {
+      return nullptr;
+    }
   }
 
   CHECK(env->req_wrap_queue()->IsEmpty());
   CHECK(env->handle_wrap_queue()->IsEmpty());
-  env->set_has_run_bootstrapping_code(true);
   return env;
 }
 

test/cctest/test_base_object_ptr.cc

@@ -14,10 +14,6 @@ using v8::Isolate;
 using v8::Local;
 using v8::Object;
 
-// Environments may come with existing BaseObject instances.
-// This variable offsets the expected BaseObject counts.
-static const int BASE_OBJECT_COUNT = 0;
-
 class BaseObjectPtrTest : public EnvironmentTestFixture {};
 
 class DummyBaseObject : public BaseObject {
@@ -51,12 +47,12 @@ TEST_F(BaseObjectPtrTest, ScopedDetached) {
   Env env_{handle_scope, argv};
   Environment* env = *env_;
 
-  EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT);
+  EXPECT_EQ(env->base_object_created_after_bootstrap(), 0);
   {
     BaseObjectPtr<DummyBaseObject> ptr = DummyBaseObject::NewDetached(env);
-    EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 1);
+    EXPECT_EQ(env->base_object_created_after_bootstrap(), 1);
   }
-  EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT);
+  EXPECT_EQ(env->base_object_created_after_bootstrap(), 0);
 }
 
 TEST_F(BaseObjectPtrTest, ScopedDetachedWithWeak) {
@@ -67,14 +63,14 @@ TEST_F(BaseObjectPtrTest, ScopedDetachedWithWeak) {
 
   BaseObjectWeakPtr<DummyBaseObject> weak_ptr;
 
-  EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT);
+  EXPECT_EQ(env->base_object_created_after_bootstrap(), 0);
   {
     BaseObjectPtr<DummyBaseObject> ptr = DummyBaseObject::NewDetached(env);
     weak_ptr = ptr;
-    EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 1);
+    EXPECT_EQ(env->base_object_created_after_bootstrap(), 1);
   }
   EXPECT_EQ(weak_ptr.get(), nullptr);
-  EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT);
+  EXPECT_EQ(env->base_object_created_after_bootstrap(), 0);
 }
 
 TEST_F(BaseObjectPtrTest, Undetached) {
@@ -86,13 +82,12 @@ TEST_F(BaseObjectPtrTest, Undetached) {
   node::AddEnvironmentCleanupHook(
       isolate_,
       [](void* arg) {
-        EXPECT_EQ(static_cast<Environment*>(arg)->base_object_count(),
-                  BASE_OBJECT_COUNT);
+        EXPECT_EQ(static_cast<Environment*>(arg)->base_object_count(), 0);
       },
       env);
 
   BaseObjectPtr<DummyBaseObject> ptr = DummyBaseObject::New(env);
-  EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 1);
+  EXPECT_EQ(env->base_object_created_after_bootstrap(), 1);
 }
 
 TEST_F(BaseObjectPtrTest, GCWeak) {
@@ -109,21 +104,21 @@ TEST_F(BaseObjectPtrTest, GCWeak) {
     weak_ptr = ptr;
     ptr->MakeWeak();
 
-    EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 1);
+    EXPECT_EQ(env->base_object_created_after_bootstrap(), 1);
     EXPECT_EQ(weak_ptr.get(), ptr.get());
     EXPECT_EQ(weak_ptr->persistent().IsWeak(), false);
 
     ptr.reset();
   }
 
-  EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 1);
+  EXPECT_EQ(env->base_object_created_after_bootstrap(), 1);
   EXPECT_NE(weak_ptr.get(), nullptr);
   EXPECT_EQ(weak_ptr->persistent().IsWeak(), true);
 
   v8::V8::SetFlagsFromString("--expose-gc");
   isolate_->RequestGarbageCollectionForTesting(Isolate::kFullGarbageCollection);
 
-  EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT);
+  EXPECT_EQ(env->base_object_created_after_bootstrap(), 0);
   EXPECT_EQ(weak_ptr.get(), nullptr);
 }
 
@@ -134,7 +129,7 @@ TEST_F(BaseObjectPtrTest, Moveable) {
   Environment* env = *env_;
 
   BaseObjectPtr<DummyBaseObject> ptr = DummyBaseObject::NewDetached(env);
-  EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 1);
+  EXPECT_EQ(env->base_object_created_after_bootstrap(), 1);
   BaseObjectWeakPtr<DummyBaseObject> weak_ptr { ptr };
   EXPECT_EQ(weak_ptr.get(), ptr.get());
 
@@ -145,12 +140,12 @@ TEST_F(BaseObjectPtrTest, Moveable) {
   BaseObjectWeakPtr<DummyBaseObject> weak_ptr2 = std::move(weak_ptr);
   EXPECT_EQ(weak_ptr2.get(), ptr2.get());
   EXPECT_EQ(weak_ptr.get(), nullptr);
-  EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 1);
+  EXPECT_EQ(env->base_object_created_after_bootstrap(), 1);
 
   ptr2.reset();
 
   EXPECT_EQ(weak_ptr2.get(), nullptr);
-  EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT);
+  EXPECT_EQ(env->base_object_created_after_bootstrap(), 0);
 }
 
 TEST_F(BaseObjectPtrTest, NestedClasses) {
@@ -174,8 +169,7 @@ TEST_F(BaseObjectPtrTest, NestedClasses) {
   node::AddEnvironmentCleanupHook(
       isolate_,
       [](void* arg) {
-        EXPECT_EQ(static_cast<Environment*>(arg)->base_object_count(),
-                  BASE_OBJECT_COUNT);
+        EXPECT_EQ(static_cast<Environment*>(arg)->base_object_count(), 0);
       },
       env);
 
@@ -184,5 +178,5 @@ TEST_F(BaseObjectPtrTest, NestedClasses) {
   obj->ptr1 = DummyBaseObject::NewDetached(env);
   obj->ptr2 = DummyBaseObject::New(env);
 
-  EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 3);
+  EXPECT_EQ(env->base_object_created_after_bootstrap(), 3);
 }