src: enhance feature access `CHECK`s during bootstrap · nodejs/node@2f4069a · GitHub

nodejs / node Public

• Notifications You must be signed in to change notification settings
• Fork 36.6k
• Star 120k

• Code
• Issues 638
• Pull requests 759
• Actions
• Projects
• Security and quality
• Insights

Additional navigation options

Commit 2f4069a

Browse filesBrowse the repository at this point in the historyBrowse files

addaleax

authored and

MylesBorins

committed

src: enhance feature access CHECKs 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>

1 parent 4b716a6 commit 2f4069a
Copy full SHA for 2f4069a

4 files changed

+9-1Lines changed: 9 additions & 1 deletion

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

• src
  • handle_wrap.cc
  • node.cc
  • node_env_var.cc
  • req_wrap-inl.h

Expand file treeCollapse file tree

Open diff view settings

Collapse file

‎src/handle_wrap.cc‎

Copy file name to clipboardExpand all lines: src/handle_wrap.cc

+1Lines changed: 1 addition & 0 deletions

Original file line number	Diff line number	Diff line change
@@ -116,6 +116,7 @@ HandleWrap::HandleWrap(Environment* env,
116	116	handle_(handle) {
117	117	handle_->data = this;
118	118	HandleScope scope(env->isolate());
	119	+ CHECK(env->has_run_bootstrapping_code());
119	120	env->handle_wrap_queue()->PushBack(this);
120	121	}
121	122

Collapse file

‎src/node.cc‎

Copy file name to clipboardExpand all lines: src/node.cc

+2-1Lines changed: 2 additions & 1 deletion

Original file line number	Diff line number	Diff line change
@@ -336,7 +336,8 @@ MaybeLocal<Value> Environment::RunBootstrapping() {
336	336
337	337	// Make sure that no request or handle is created during bootstrap -
338	338	// if necessary those should be done in pre-execution.
339		- // TODO(joyeecheung): print handles/requests before aborting
	339	+ // Usually, doing so would trigger the checks present in the ReqWrap and
	340	+ // HandleWrap classes, so this is only a consistency check.
340	341	CHECK(req_wrap_queue()->IsEmpty());
341	342	CHECK(handle_wrap_queue()->IsEmpty());
342	343

Collapse file

‎src/node_env_var.cc‎

Copy file name to clipboardExpand all lines: src/node_env_var.cc

+5Lines changed: 5 additions & 0 deletions

Original file line number	Diff line number	Diff line change
@@ -255,6 +255,7 @@ Maybe<bool> KVStore::AssignFromObject(Local<Context> context,
255	255	static void EnvGetter(Local<Name> property,
256	256	const PropertyCallbackInfo<Value>& info) {
257	257	Environment* env = Environment::GetCurrent(info);
	258	+ CHECK(env->has_run_bootstrapping_code());
258	259	if (property->IsSymbol()) {
259	260	return info.GetReturnValue().SetUndefined();
260	261	}
@@ -270,6 +271,7 @@ static void EnvSetter(Local<Name> property,
270	271	Local<Value> value,
271	272	const PropertyCallbackInfo<Value>& info) {
272	273	Environment* env = Environment::GetCurrent(info);
	274	+ CHECK(env->has_run_bootstrapping_code());
273	275	// calling env->EmitProcessEnvWarning() sets a variable indicating that
274	276	// warnings have been emitted. It should be called last after other
275	277	// conditions leading to a warning have been met.
@@ -303,6 +305,7 @@ static void EnvSetter(Local<Name> property,
303	305	static void EnvQuery(Local<Name> property,
304	306	const PropertyCallbackInfo<Integer>& info) {
305	307	Environment* env = Environment::GetCurrent(info);
	308	+ CHECK(env->has_run_bootstrapping_code());
306	309	if (property->IsString()) {
307	310	int32_t rc = env->env_vars()->Query(env->isolate(), property.As<String>());
308	311	if (rc != -1) info.GetReturnValue().Set(rc);
@@ -312,6 +315,7 @@ static void EnvQuery(Local<Name> property,
312	315	static void EnvDeleter(Local<Name> property,
313	316	const PropertyCallbackInfo<Boolean>& info) {
314	317	Environment* env = Environment::GetCurrent(info);
	318	+ CHECK(env->has_run_bootstrapping_code());
315	319	if (property->IsString()) {
316	320	env->env_vars()->Delete(env->isolate(), property.As<String>());
317	321	}
@@ -323,6 +327,7 @@ static void EnvDeleter(Local<Name> property,
323	327
324	328	static void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
325	329	Environment* env = Environment::GetCurrent(info);
	330	+ CHECK(env->has_run_bootstrapping_code());
326	331
327	332	info.GetReturnValue().Set(
328	333	env->env_vars()->Enumerate(env->isolate()));

Collapse file

‎src/req_wrap-inl.h‎

Copy file name to clipboardExpand all lines: src/req_wrap-inl.h

+1Lines changed: 1 addition & 0 deletions

Original file line number	Diff line number	Diff line change
@@ -10,6 +10,7 @@
10	10	namespace node {
11	11
12	12	ReqWrapBase::ReqWrapBase(Environment* env) {
	13	+ CHECK(env->has_run_bootstrapping_code());
13	14	env->req_wrap_queue()->PushBack(this);
14	15	}
15	16

0 commit comments

Comments

0 (0)