src: refactor Environment::GetCurrent(isolate) usage

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()) &&

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]) {

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);
 }

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());
 }
 

src/inspector/main_thread_interface.cc

@@ -287,6 +287,14 @@ void MainThreadInterface::DispatchMessages() {
       MessageQueue::value_type task;
       std::swap(dispatching_message_queue_.front(), task);
       dispatching_message_queue_.pop_front();
+
+      // TODO(addaleax): The V8 inspector code currently sometimes allocates
+      // handles that leak to the outside scope, rendering a HandleScope here
+      // necessary. This handle scope can be removed/turned into a
+      // SealHandleScope once/if
+      // https://chromium-review.googlesource.com/c/v8/v8/+/1484304 makes it
+      // into our copy of V8, maybe guarded with #ifdef DEBUG if we want.
+      v8::HandleScope handle_scope(isolate_);
       task->Call(this);
     }
   } while (had_messages);

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(

src/node_perf.cc

@@ -333,10 +333,9 @@ inline Local<Value> GetName(Local<Function> fn) {
 // execution.
 void TimerFunctionCall(const FunctionCallbackInfo<Value>& args) {
   Isolate* isolate = args.GetIsolate();
-  HandleScope scope(isolate);
-  Environment* env = Environment::GetCurrent(isolate);
+  Local<Context> context = isolate->GetCurrentContext();
+  Environment* env = Environment::GetCurrent(context);
   CHECK_NOT_NULL(env);
-  Local<Context> context = env->context();
   Local<Function> fn = args.Data().As<Function>();
   size_t count = args.Length();
   size_t idx;

src/node_platform.cc

@@ -8,11 +8,11 @@
 
 namespace node {
 
-using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
 using v8::Object;
 using v8::Platform;
+using v8::SealHandleScope;
 using v8::Task;
 using node::tracing::TracingController;
 
@@ -332,11 +332,17 @@ int NodePlatform::NumberOfWorkerThreads() {
 
 void PerIsolatePlatformData::RunForegroundTask(std::unique_ptr<Task> task) {
   Isolate* isolate = Isolate::GetCurrent();
-  HandleScope scope(isolate);
+#ifdef DEBUG
+  SealHandleScope scope(isolate);
+#endif
   Environment* env = Environment::GetCurrent(isolate);
-  InternalCallbackScope cb_scope(env, Local<Object>(), { 0, 0 },
-                                 InternalCallbackScope::kAllowEmptyResource);
-  task->Run();
+  if (env != nullptr) {
+    InternalCallbackScope cb_scope(env, Local<Object>(), { 0, 0 },
+                                   InternalCallbackScope::kAllowEmptyResource);
+    task->Run();
+  } else {
+    task->Run();
+  }
 }
 
 void PerIsolatePlatformData::DeleteFromScheduledTasks(DelayedTask* task) {