Skip to content
Permalink
Browse files

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: #30452
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax authored and MylesBorins committed Nov 12, 2019
1 parent 4b716a6 commit 2f4069a932d4373d618d6ecb7ff8dbe30807502f
Showing with 9 additions and 1 deletion.
  1. +1 −0 src/handle_wrap.cc
  2. +2 −1 src/node.cc
  3. +5 −0 src/node_env_var.cc
  4. +1 −0 src/req_wrap-inl.h
@@ -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);
}

@@ -336,7 +336,8 @@ MaybeLocal<Value> 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());

@@ -255,6 +255,7 @@ Maybe<bool> KVStore::AssignFromObject(Local<Context> context,
static void EnvGetter(Local<Name> property,
const PropertyCallbackInfo<Value>& 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<Name> property,
Local<Value> value,
const PropertyCallbackInfo<Value>& 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<Name> property,
static void EnvQuery(Local<Name> property,
const PropertyCallbackInfo<Integer>& 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<String>());
if (rc != -1) info.GetReturnValue().Set(rc);
@@ -312,6 +315,7 @@ static void EnvQuery(Local<Name> property,
static void EnvDeleter(Local<Name> property,
const PropertyCallbackInfo<Boolean>& info) {
Environment* env = Environment::GetCurrent(info);
CHECK(env->has_run_bootstrapping_code());
if (property->IsString()) {
env->env_vars()->Delete(env->isolate(), property.As<String>());
}
@@ -323,6 +327,7 @@ static void EnvDeleter(Local<Name> property,

static void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
Environment* env = Environment::GetCurrent(info);
CHECK(env->has_run_bootstrapping_code());

info.GetReturnValue().Set(
env->env_vars()->Enumerate(env->isolate()));
@@ -10,6 +10,7 @@
namespace node {

ReqWrapBase::ReqWrapBase(Environment* env) {
CHECK(env->has_run_bootstrapping_code());
env->req_wrap_queue()->PushBack(this);
}

0 comments on commit 2f4069a

Please sign in to comment.
You can’t perform that action at this time.