From 716ec00883364be48f9d42b42b9a36fa1550b8c8 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sun, 24 Feb 2019 21:52:14 +0100
Subject: [PATCH] src: refactor `Environment::GetCurrent(isolate)` usage

Do not require an explicit `HandleScope`, or the ability to create
one, when using `Environment::GetCurrent()`.

`isolate->InContext()` is used as an indicator that it is probably
okay to create a `HandleScope`, see also the short discussion in
https://github.com/nodejs/node/pull/25775#pullrequestreview-197371049.

PR-URL: https://github.com/nodejs/node/pull/26376
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/api/environment.cc |  5 ++++-
 src/api/exceptions.cc  |  3 +++
 src/api/hooks.cc       | 11 ++++-------
 src/env-inl.h          |  2 ++
 src/node_errors.cc     |  1 -
 5 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/api/environment.cc b/src/api/environment.cc
index 786ca5021b5ec7..27e580133505a2 100644
--- a/src/api/environment.cc
+++ b/src/api/environment.cc
@@ -23,6 +23,7 @@ using v8::MaybeLocal;
 using v8::Message;
 using v8::MicrotasksPolicy;
 using v8::ObjectTemplate;
+using v8::SealHandleScope;
 using v8::String;
 using v8::Value;
 
@@ -34,7 +35,9 @@ static bool AllowWasmCodeGenerationCallback(Local<Context> context,
 }
 
 static bool ShouldAbortOnUncaughtException(Isolate* isolate) {
-  HandleScope scope(isolate);
+#ifdef DEBUG
+  SealHandleScope scope(isolate);
+#endif
   Environment* env = Environment::GetCurrent(isolate);
   return env != nullptr &&
          (env->is_main_thread() || !env->is_stopping_worker()) &&
diff --git a/src/api/exceptions.cc b/src/api/exceptions.cc
index 897a4de365283b..ceac9374082ad8 100644
--- a/src/api/exceptions.cc
+++ b/src/api/exceptions.cc
@@ -28,6 +28,7 @@ Local<Value> ErrnoException(Isolate* isolate,
                             const char* msg,
                             const char* path) {
   Environment* env = Environment::GetCurrent(isolate);
+  CHECK_NOT_NULL(env);
 
   Local<Value> e;
   Local<String> estring = OneByteString(isolate, errors::errno_string(errorno));
@@ -99,6 +100,7 @@ Local<Value> UVException(Isolate* isolate,
                          const char* path,
                          const char* dest) {
   Environment* env = Environment::GetCurrent(isolate);
+  CHECK_NOT_NULL(env);
 
   if (!msg || !msg[0])
     msg = uv_strerror(errorno);
@@ -187,6 +189,7 @@ Local<Value> WinapiErrnoException(Isolate* isolate,
                                   const char* msg,
                                   const char* path) {
   Environment* env = Environment::GetCurrent(isolate);
+  CHECK_NOT_NULL(env);
   Local<Value> e;
   bool must_free = false;
   if (!msg || !msg[0]) {
diff --git a/src/api/hooks.cc b/src/api/hooks.cc
index b54292638ddf95..aa647778d51537 100644
--- a/src/api/hooks.cc
+++ b/src/api/hooks.cc
@@ -11,6 +11,7 @@ using v8::Integer;
 using v8::Isolate;
 using v8::Local;
 using v8::Object;
+using v8::SealHandleScope;
 using v8::String;
 using v8::Value;
 using v8::NewStringType;
@@ -88,16 +89,12 @@ void RemoveEnvironmentCleanupHook(Isolate* isolate,
 }
 
 async_id AsyncHooksGetExecutionAsyncId(Isolate* isolate) {
-  // Environment::GetCurrent() allocates a Local<> handle.
-  HandleScope handle_scope(isolate);
   Environment* env = Environment::GetCurrent(isolate);
   if (env == nullptr) return -1;
   return env->execution_async_id();
 }
 
 async_id AsyncHooksGetTriggerAsyncId(Isolate* isolate) {
-  // Environment::GetCurrent() allocates a Local<> handle.
-  HandleScope handle_scope(isolate);
   Environment* env = Environment::GetCurrent(isolate);
   if (env == nullptr) return -1;
   return env->trigger_async_id();
@@ -119,7 +116,9 @@ async_context EmitAsyncInit(Isolate* isolate,
                             Local<Object> resource,
                             Local<String> name,
                             async_id trigger_async_id) {
-  HandleScope handle_scope(isolate);
+#ifdef DEBUG
+  SealHandleScope handle_scope(isolate);
+#endif
   Environment* env = Environment::GetCurrent(isolate);
   CHECK_NOT_NULL(env);
 
@@ -140,8 +139,6 @@ async_context EmitAsyncInit(Isolate* isolate,
 }
 
 void EmitAsyncDestroy(Isolate* isolate, async_context asyncContext) {
-  // Environment::GetCurrent() allocates a Local<> handle.
-  HandleScope handle_scope(isolate);
   AsyncWrap::EmitDestroy(
       Environment::GetCurrent(isolate), asyncContext.async_id);
 }
diff --git a/src/env-inl.h b/src/env-inl.h
index 6c4120fc2e406b..2a560f71d24bc1 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -296,6 +296,8 @@ inline void Environment::AssignToContext(v8::Local<v8::Context> context,
 }
 
 inline Environment* Environment::GetCurrent(v8::Isolate* isolate) {
+  if (UNLIKELY(!isolate->InContext())) return nullptr;
+  v8::HandleScope handle_scope(isolate);
   return GetCurrent(isolate->GetCurrentContext());
 }
 
diff --git a/src/node_errors.cc b/src/node_errors.cc
index 1c15b5558457d4..5c1e227966c113 100644
--- a/src/node_errors.cc
+++ b/src/node_errors.cc
@@ -314,7 +314,6 @@ void OnFatalError(const char* location, const char* message) {
   }
 #ifdef NODE_REPORT
   Isolate* isolate = Isolate::GetCurrent();
-  HandleScope handle_scope(isolate);
   Environment* env = Environment::GetCurrent(isolate);
   if (env == nullptr || env->isolate_data()->options()->report_on_fatalerror) {
     report::TriggerNodeReport(