Cancel EnqueueMicrotask on detached contexts

Context::microtask_context can be null after v8::Context::DetachGlobal
is called, and that should cancel microtasks that are associated to
the detached context.
However, there are several callers left without the null check to the
microtask queue, and that causes crashes.

This CL adds the null check and cancellation as the crash fix.

Bug: chromium:937784
Change-Id: Ie8d107f28f200cee6e75798e3f72c5ed7a2a461c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1545139
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60623}

src/api.cc

@@ -8555,7 +8555,8 @@ void Isolate::EnqueueMicrotask(Local<Function> v8_function) {
   if (!i::JSReceiver::GetContextForMicrotask(function).ToHandle(
           &handler_context))
     handler_context = isolate->native_context();
-  handler_context->microtask_queue()->EnqueueMicrotask(this, v8_function);
+  MicrotaskQueue* microtask_queue = handler_context->microtask_queue();
+  if (microtask_queue) microtask_queue->EnqueueMicrotask(this, v8_function);
 }
 
 void Isolate::EnqueueMicrotask(MicrotaskCallback callback, void* data) {

src/objects.cc

@@ -6112,7 +6112,9 @@ MaybeHandle<Object> JSPromise::Resolve(Handle<JSPromise> promise,
                         promise)
         .Check();
   }
-  isolate->native_context()->microtask_queue()->EnqueueMicrotask(*task);
+  MicrotaskQueue* microtask_queue =
+      isolate->native_context()->microtask_queue();
+  if (microtask_queue) microtask_queue->EnqueueMicrotask(*task);
 
   // 13. Return undefined.
   return isolate->factory()->undefined_value();
@@ -6202,8 +6204,11 @@ Handle<Object> JSPromise::TriggerPromiseReactions(Isolate* isolate,
               PromiseRejectReactionJobTask::kPromiseOrCapabilityOffset));
     }
 
-    handler_context->microtask_queue()->EnqueueMicrotask(
-        *Handle<PromiseReactionJobTask>::cast(task));
+    MicrotaskQueue* microtask_queue = handler_context->microtask_queue();
+    if (microtask_queue) {
+      microtask_queue->EnqueueMicrotask(
+          *Handle<PromiseReactionJobTask>::cast(task));
+    }
   }
 
   return isolate->factory()->undefined_value();

src/objects/js-promise.h

@@ -53,8 +53,8 @@ class JSPromise : public JSObject {
   void set_status(Promise::PromiseState status);
 
   // ES section #sec-fulfillpromise
-  static Handle<Object> Fulfill(Handle<JSPromise> promise,
-                                Handle<Object> value);
+  V8_EXPORT_PRIVATE static Handle<Object> Fulfill(Handle<JSPromise> promise,
+                                                  Handle<Object> value);
   // ES section #sec-rejectpromise
   static Handle<Object> Reject(Handle<JSPromise> promise, Handle<Object> reason,
                                bool debug_event = true);

src/runtime/runtime-promise.cc

@@ -79,7 +79,9 @@ RUNTIME_FUNCTION(Runtime_EnqueueMicrotask) {
 
   Handle<CallableTask> microtask = isolate->factory()->NewCallableTask(
       function, handle(function->native_context(), isolate));
-  function->native_context()->microtask_queue()->EnqueueMicrotask(*microtask);
+  MicrotaskQueue* microtask_queue =
+      function->native_context()->microtask_queue();
+  if (microtask_queue) microtask_queue->EnqueueMicrotask(*microtask);
   return ReadOnlyRoots(isolate).undefined_value();
 }
 

test/unittests/microtask-queue-unittest.cc

@@ -40,6 +40,7 @@ class WithFinalizationGroupMixin : public TMixin {
     save_flags_ = new SaveFlags();
     FLAG_harmony_weak_refs = true;
     FLAG_expose_gc = true;
+    FLAG_allow_natives_syntax = true;
     TMixin::SetUpTestCase();
   }
 
@@ -541,5 +542,47 @@ TEST_F(MicrotaskQueueTest, DetachGlobal_Chain) {
       Object::GetElement(isolate(), result, 0).ToHandleChecked()->IsTrue());
 }
 
+TEST_F(MicrotaskQueueTest, DetachGlobal_InactiveHandler) {
+  Local<v8::Context> sub_context = v8::Context::New(v8_isolate());
+  Utils::OpenHandle(*sub_context)
+      ->native_context()
+      ->set_microtask_queue(microtask_queue());
+
+  Handle<JSArray> result;
+  Handle<JSFunction> stale_handler;
+  Handle<JSPromise> stale_promise;
+  {
+    v8::Context::Scope scope(sub_context);
+    result = RunJS<JSArray>("var result = [false, false]; result");
+    stale_handler = RunJS<JSFunction>("() => { result[0] = true; }");
+    stale_promise = RunJS<JSPromise>(
+        "var stale_promise = new Promise(()=>{});"
+        "stale_promise");
+    RunJS("stale_promise.then(() => { result [1] = true; });");
+  }
+  sub_context->DetachGlobal();
+  sub_context.Clear();
+
+  // The context of |stale_handler| and |stale_promise| is detached at this
+  // point.
+  // Ensure that resolution handling for |stale_handler| is cancelled without
+  // crash. Also, the resolution of |stale_promise| is also cancelled.
+
+  SetGlobalProperty("stale_handler", Utils::ToLocal(stale_handler));
+  RunJS("%EnqueueMicrotask(stale_handler)");
+
+  v8_isolate()->EnqueueMicrotask(Utils::ToLocal(stale_handler));
+
+  JSPromise::Fulfill(
+      stale_promise,
+      handle(ReadOnlyRoots(isolate()).undefined_value(), isolate()));
+
+  microtask_queue()->RunMicrotasks(isolate());
+  EXPECT_TRUE(
+      Object::GetElement(isolate(), result, 0).ToHandleChecked()->IsFalse());
+  EXPECT_TRUE(
+      Object::GetElement(isolate(), result, 1).ToHandleChecked()->IsFalse());
+}
+
 }  // namespace internal
 }  // namespace v8