src: run native immediates during Environment cleanup

src/env.cc

@@ -527,6 +527,11 @@ void Environment::RegisterHandleCleanups() {
 }
 
 void Environment::CleanupHandles() {
+  Isolate::DisallowJavascriptExecutionScope disallow_js(isolate(),
+      Isolate::DisallowJavascriptExecutionScope::THROW_ON_FAILURE);
+
+  RunAndClearNativeImmediates(true /* skip SetUnrefImmediate()s */);
+
   for (ReqWrapBase* request : req_wrap_queue_)
     request->Cancel();
 
@@ -642,7 +647,7 @@ void Environment::AtExit(void (*cb)(void* arg), void* arg) {
   at_exit_functions_.push_front(ExitCallback{cb, arg});
 }
 
-void Environment::RunAndClearNativeImmediates() {
+void Environment::RunAndClearNativeImmediates(bool only_refed) {
   TraceEventScope trace_scope(TRACING_CATEGORY_NODE1(environment),
                               "RunAndClearNativeImmediates", this);
   size_t ref_count = 0;
@@ -659,9 +664,11 @@ void Environment::RunAndClearNativeImmediates() {
       if (head->is_refed())
         ref_count++;
 
-      head->Call(this);
+      if (head->is_refed() || !only_refed)
+        head->Call(this);
+
       if (UNLIKELY(try_catch.HasCaught())) {
-        if (!try_catch.HasTerminated())
+        if (!try_catch.HasTerminated() && can_call_into_js())
           errors::TriggerUncaughtException(isolate(), try_catch);
 
         // We are done with the current callback. Move one iteration along,

src/env.h

@@ -1415,7 +1415,7 @@ class Environment : public MemoryRetainer {
   std::unique_ptr<NativeImmediateCallback> native_immediate_callbacks_head_;
   NativeImmediateCallback* native_immediate_callbacks_tail_ = nullptr;
 
-  void RunAndClearNativeImmediates();
+  void RunAndClearNativeImmediates(bool only_refed = false);
   static void CheckImmediate(uv_check_t* handle);
 
   // Use an unordered_set, so that we have efficient insertion and removal.

src/node_errors.cc

@@ -278,6 +278,9 @@ static void ReportFatalException(Environment* env,
                                  Local<Value> error,
                                  Local<Message> message,
                                  EnhanceFatalException enhance_stack) {
+  if (!env->can_call_into_js())
+    enhance_stack = EnhanceFatalException::kDontEnhance;
+
   Isolate* isolate = env->isolate();
   CHECK(!error.IsEmpty());
   CHECK(!message.IsEmpty());
@@ -956,7 +959,7 @@ void TriggerUncaughtException(Isolate* isolate,
   }
 
   MaybeLocal<Value> handled;
-  {
+  if (env->can_call_into_js()) {
     // We do not expect the global uncaught exception itself to throw any more
     // exceptions. If it does, exit the current Node.js instance.
     errors::TryCatchScope try_catch(env,

src/node_process_events.cc

@@ -34,6 +34,8 @@ Maybe<bool> ProcessEmitWarningGeneric(Environment* env,
                                       const char* warning,
                                       const char* type,
                                       const char* code) {
+  if (!env->can_call_into_js()) return Just(false);
+
   HandleScope handle_scope(env->isolate());
   Context::Scope context_scope(env->context());
 

src/stream_pipe.cc

@@ -42,7 +42,7 @@ StreamPipe::StreamPipe(StreamBase* source,
 }
 
 StreamPipe::~StreamPipe() {
-  Unpipe();
+  Unpipe(true);
 }
 
 StreamBase* StreamPipe::source() {
@@ -53,7 +53,7 @@ StreamBase* StreamPipe::sink() {
   return static_cast<StreamBase*>(writable_listener_.stream());
 }
 
-void StreamPipe::Unpipe() {
+void StreamPipe::Unpipe(bool is_in_deletion) {
   if (is_closed_)
     return;
 
@@ -68,11 +68,13 @@ void StreamPipe::Unpipe() {
   source()->RemoveStreamListener(&readable_listener_);
   sink()->RemoveStreamListener(&writable_listener_);
 
+  if (is_in_deletion) return;
+
   // Delay the JS-facing part with SetImmediate, because this might be from
   // inside the garbage collector, so we can’t run JS here.
   HandleScope handle_scope(env()->isolate());
   BaseObjectPtr<StreamPipe> strong_ref{this};
-  env()->SetImmediate([this](Environment* env) {
+  env()->SetImmediate([this, strong_ref](Environment* env) {
     HandleScope handle_scope(env->isolate());
     Context::Scope context_scope(env->context());
     Local<Object> object = this->object();

src/stream_pipe.h

@@ -12,7 +12,7 @@ class StreamPipe : public AsyncWrap {
   StreamPipe(StreamBase* source, StreamBase* sink, v8::Local<v8::Object> obj);
   ~StreamPipe() override;
 
-  void Unpipe();
+  void Unpipe(bool is_in_deletion = false);
 
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Start(const v8::FunctionCallbackInfo<v8::Value>& args);

test/cctest/test_environment.cc

@@ -185,3 +185,26 @@ static void at_exit_js(void* arg) {
   assert(obj->IsObject());
   called_at_exit_js = true;
 }
+
+TEST_F(EnvironmentTest, SetImmediateCleanup) {
+  int called = 0;
+  int called_unref = 0;
+
+  {
+    const v8::HandleScope handle_scope(isolate_);
+    const Argv argv;
+    Env env {handle_scope, argv};
+
+    (*env)->SetImmediate([&](node::Environment* env_arg) {
+      EXPECT_EQ(env_arg, *env);
+      called++;
+    });
+    (*env)->SetUnrefImmediate([&](node::Environment* env_arg) {
+      EXPECT_EQ(env_arg, *env);
+      called_unref++;
+    });
+  }
+
+  EXPECT_EQ(called, 1);
+  EXPECT_EQ(called_unref, 0);
+}