diff --git a/src/env-inl.h b/src/env-inl.h index 98badcd0362627..95b818373b9a0a 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -784,8 +784,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 { @@ -1089,8 +1093,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 str) { diff --git a/src/env.cc b/src/env.cc index 69d5a33a945d9f..16c6fcc3db4749 100644 --- a/src/env.cc +++ b/src/env.cc @@ -422,10 +422,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, @@ -468,10 +464,6 @@ void Environment::InitializeMainContext(Local 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() { @@ -662,7 +654,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() || diff --git a/src/env.h b/src/env.h index 38c087b650e3ac..f2fc681f56ed69 100644 --- a/src/env.h +++ b/src/env.h @@ -1132,7 +1132,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); @@ -1318,6 +1318,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; } @@ -1511,7 +1512,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 unmanaged_fds_; diff --git a/src/node.cc b/src/node.cc index efb4876002f774..decaa755e39f12 100644 --- a/src/node.cc +++ b/src/node.cc @@ -415,7 +415,7 @@ MaybeLocal Environment::RunBootstrapping() { CHECK(req_wrap_queue()->IsEmpty()); CHECK(handle_wrap_queue()->IsEmpty()); - set_has_run_bootstrapping_code(true); + DoneBootstrapping(); return scope.Escape(result); } diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index 84d39831dcd1f7..37d9dbb9b75525 100644 --- a/src/node_main_instance.cc +++ b/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; } diff --git a/test/cctest/test_base_object_ptr.cc b/test/cctest/test_base_object_ptr.cc index a3a8df98a9efc7..52a1e2aff79dde 100644 --- a/test/cctest/test_base_object_ptr.cc +++ b/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 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 weak_ptr; - EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT); + EXPECT_EQ(env->base_object_created_after_bootstrap(), 0); { BaseObjectPtr 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(arg)->base_object_count(), - BASE_OBJECT_COUNT); + EXPECT_EQ(static_cast(arg)->base_object_count(), 0); }, env); BaseObjectPtr 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 ptr = DummyBaseObject::NewDetached(env); - EXPECT_EQ(env->base_object_count(), BASE_OBJECT_COUNT + 1); + EXPECT_EQ(env->base_object_created_after_bootstrap(), 1); BaseObjectWeakPtr weak_ptr { ptr }; EXPECT_EQ(weak_ptr.get(), ptr.get()); @@ -145,12 +140,12 @@ TEST_F(BaseObjectPtrTest, Moveable) { BaseObjectWeakPtr 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(arg)->base_object_count(), - BASE_OBJECT_COUNT); + EXPECT_EQ(static_cast(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); }