From 592d51cb23ff9de1737a827bd023b8a08e2a49a6 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
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 <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>
---
 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 5a8e6ea8c07982..5379b42b578c1c 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -344,7 +344,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());
 
diff --git a/src/node_env_var.cc b/src/node_env_var.cc
index 9d229ccf4e5f8b..40c0515a3dc2e2 100644
--- a/src/node_env_var.cc
+++ b/src/node_env_var.cc
@@ -272,6 +272,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();
   }
@@ -287,6 +288,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.
@@ -320,6 +322,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);
@@ -329,6 +332,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>());
   }
@@ -340,6 +344,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()));
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);
 }