From e0982f754d9295878ef9a21ce0f6b8dccbc43ed3 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 12 Nov 2019 18:33:37 +0000 Subject: [PATCH] src: enhance feature access `CHECK`s during bootstrap This adds `CHECK`s verifying that bootstrapping has finished before environment variables are accessed or handles/requests are created. The latter complements a pair of existent checks, but fails earlier and thus gives information about the call site, effectively addressing the TODO comment there. PR-URL: https://github.com/nodejs/node/pull/30452 Reviewed-By: Colin Ihrig Reviewed-By: David Carlier Reviewed-By: Joyee Cheung Reviewed-By: James M Snell --- src/handle_wrap.cc | 1 + src/node.cc | 3 ++- src/node_env_var.cc | 5 +++++ src/req_wrap-inl.h | 1 + 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/handle_wrap.cc b/src/handle_wrap.cc index 888640e9493d8e..fc84ca19bb2517 100644 --- a/src/handle_wrap.cc +++ b/src/handle_wrap.cc @@ -116,6 +116,7 @@ HandleWrap::HandleWrap(Environment* env, handle_(handle) { handle_->data = this; HandleScope scope(env->isolate()); + CHECK(env->has_run_bootstrapping_code()); env->handle_wrap_queue()->PushBack(this); } diff --git a/src/node.cc b/src/node.cc index fc6b919ece2fed..9d182d2aeb48c8 100644 --- a/src/node.cc +++ b/src/node.cc @@ -336,7 +336,8 @@ MaybeLocal Environment::RunBootstrapping() { // Make sure that no request or handle is created during bootstrap - // if necessary those should be done in pre-execution. - // TODO(joyeecheung): print handles/requests before aborting + // Usually, doing so would trigger the checks present in the ReqWrap and + // HandleWrap classes, so this is only a consistency check. CHECK(req_wrap_queue()->IsEmpty()); CHECK(handle_wrap_queue()->IsEmpty()); diff --git a/src/node_env_var.cc b/src/node_env_var.cc index 50cbd777d32b6d..a551849ff4bb82 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -255,6 +255,7 @@ Maybe KVStore::AssignFromObject(Local context, static void EnvGetter(Local property, const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); + CHECK(env->has_run_bootstrapping_code()); if (property->IsSymbol()) { return info.GetReturnValue().SetUndefined(); } @@ -270,6 +271,7 @@ static void EnvSetter(Local property, Local value, const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); + CHECK(env->has_run_bootstrapping_code()); // calling env->EmitProcessEnvWarning() sets a variable indicating that // warnings have been emitted. It should be called last after other // conditions leading to a warning have been met. @@ -303,6 +305,7 @@ static void EnvSetter(Local property, static void EnvQuery(Local property, const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); + CHECK(env->has_run_bootstrapping_code()); if (property->IsString()) { int32_t rc = env->env_vars()->Query(env->isolate(), property.As()); if (rc != -1) info.GetReturnValue().Set(rc); @@ -312,6 +315,7 @@ static void EnvQuery(Local property, static void EnvDeleter(Local property, const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); + CHECK(env->has_run_bootstrapping_code()); if (property->IsString()) { env->env_vars()->Delete(env->isolate(), property.As()); } @@ -323,6 +327,7 @@ static void EnvDeleter(Local property, static void EnvEnumerator(const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); + CHECK(env->has_run_bootstrapping_code()); info.GetReturnValue().Set( env->env_vars()->Enumerate(env->isolate())); diff --git a/src/req_wrap-inl.h b/src/req_wrap-inl.h index cf89fb58a7f6fc..4fa4d0cf217069 100644 --- a/src/req_wrap-inl.h +++ b/src/req_wrap-inl.h @@ -10,6 +10,7 @@ namespace node { ReqWrapBase::ReqWrapBase(Environment* env) { + CHECK(env->has_run_bootstrapping_code()); env->req_wrap_queue()->PushBack(this); }