deps: V8: cherry-pick 9f0f2cb7f08d

Original commit message:

    [weakrefs] Call Isolate::ClearKeptObjects() as part of microtask checkpoint

    In the spec, WeakRefs that are dereferenced are kept alive until there's
    no JS on the stack, and then the host is expected to call
    ClearKeptObjects to clear those strong references [1]. HTML calls
    ClearKeptObjects at the end of a PerformMicrotaskCheckpoint [2].

    In V8, leaving this up to the embedder is error prone in the same way
    the deprecated FinalizationGroup callback APIs were error prone: it
    depends on the embedder doing the right thing. This CL moves the call to
    ClearKeptObjects to be after running of microtasks within V8.

    However, the Isolate::ClearKeptObjects API should not be removed or
    deprecated in case an embedder uses an entirely custom MicrotaskQueue
    implementation and invokes MicrotaskQueue::PerformCheckpoint manually.

    [1] https://tc39.es/proposal-weakrefs/#sec-clear-kept-objects
    [2] whatwg/html#4571

    Bug: v8:8179
    Change-Id: Ie243804157b56241ca69ed8fad300e839a0c9f75
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2055967
    Commit-Queue: Shu-yu Guo <[email protected]>
    Reviewed-by: Ulan Degenbaev <[email protected]>
    Reviewed-by: Ross McIlroy <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#66327}

Refs: v8/v8@9f0f2cb

PR-URL: #32885
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>

Author: addaleax

common.gypi

@@ -35,7 +35,7 @@
 
     # Reset this number to 0 on major V8 upgrades.
     # Increment by one for each non-official patch applied to deps/v8.
-    'v8_embedder_string': '-node.20',
+    'v8_embedder_string': '-node.21',
 
     ##### V8 defaults for Node.js #####
 

deps/v8/include/v8.h

@@ -7241,10 +7241,10 @@ typedef void (*MicrotasksCompletedCallback)(Isolate*);
 typedef void (*MicrotasksCompletedCallbackWithData)(Isolate*, void*);
 typedef void (*MicrotaskCallback)(void* data);
 
-
 /**
  * Policy for running microtasks:
- *   - explicit: microtasks are invoked with Isolate::RunMicrotasks() method;
+ *   - explicit: microtasks are invoked with the
+ *               Isolate::PerformMicrotaskCheckpoint() method;
  *   - scoped: microtasks invocation is controlled by MicrotasksScope objects;
  *   - auto: microtasks are invoked when the script call depth decrements
  *           to zero.
@@ -7335,7 +7335,7 @@ class V8_EXPORT MicrotaskQueue {
 };
 
 /**
- * This scope is used to control microtasks when kScopeMicrotasksInvocation
+ * This scope is used to control microtasks when MicrotasksPolicy::kScoped
  * is used on Isolate. In this mode every non-primitive call to V8 should be
  * done inside some MicrotasksScope.
  * Microtasks are executed when topmost MicrotasksScope marked as kRunMicrotasks
@@ -8416,10 +8416,13 @@ class V8_EXPORT Isolate {
    * objects are originally built when a WeakRef is created or
    * successfully dereferenced.
    *
-   * The embedder is expected to call this when a synchronous sequence
-   * of ECMAScript execution completes. It's the embedders
-   * responsiblity to make this call at a time which does not
-   * interrupt synchronous ECMAScript code execution.
+   * This is invoked automatically after microtasks are run. See
+   * MicrotasksPolicy for when microtasks are run.
+   *
+   * This needs to be manually invoked only if the embedder is manually running
+   * microtasks via a custom MicrotaskQueue class's PerformCheckpoint. In that
+   * case, it is the embedder's responsibility to make this call at a time which
+   * does not interrupt synchronous ECMAScript code execution.
    */
   void ClearKeptObjects();
 
@@ -8944,10 +8947,18 @@ class V8_EXPORT Isolate {
   void SetPromiseRejectCallback(PromiseRejectCallback callback);
 
   /**
-   * Runs the default MicrotaskQueue until it gets empty.
-   * Any exceptions thrown by microtask callbacks are swallowed.
+   * An alias for PerformMicrotaskCheckpoint.
+   */
+  V8_DEPRECATE_SOON("Use PerformMicrotaskCheckpoint.")
+  void RunMicrotasks() { PerformMicrotaskCheckpoint(); }
+
+  /**
+   * Runs the default MicrotaskQueue until it gets empty and perform other
+   * microtask checkpoint steps, such as calling ClearKeptObjects. Asserts that
+   * the MicrotasksPolicy is not kScoped. Any exceptions thrown by microtask
+   * callbacks are swallowed.
    */
-  void RunMicrotasks();
+  void PerformMicrotaskCheckpoint();
 
   /**
    * Enqueues the callback to the default MicrotaskQueue

deps/v8/src/api/api.cc

@@ -309,6 +309,7 @@ class CallDepthScope {
 #ifdef V8_CHECK_MICROTASKS_SCOPES_CONSISTENCY
     if (do_callback) CheckMicrotasksScopesConsistency(microtask_queue);
 #endif
+    DCHECK(CheckKeptObjectsClearedAfterMicrotaskCheckpoint(microtask_queue));
     isolate_->set_next_v8_call_is_safe_for_termination(safe_for_termination_);
   }
 
@@ -323,6 +324,15 @@ class CallDepthScope {
   }
 
  private:
+  bool CheckKeptObjectsClearedAfterMicrotaskCheckpoint(
+      i::MicrotaskQueue* microtask_queue) {
+    bool did_perform_microtask_checkpoint =
+        do_callback && microtask_queue &&
+        microtask_queue->microtasks_policy() == MicrotasksPolicy::kAuto;
+    return !did_perform_microtask_checkpoint ||
+           isolate_->heap()->weak_refs_keep_during_job().IsUndefined(isolate_);
+  }
+
   i::Isolate* const isolate_;
   Local<Context> context_;
   bool escaped_;
@@ -8588,10 +8598,10 @@ void Isolate::SetPromiseRejectCallback(PromiseRejectCallback callback) {
   isolate->SetPromiseRejectCallback(callback);
 }
 
-void Isolate::RunMicrotasks() {
+void Isolate::PerformMicrotaskCheckpoint() {
   DCHECK_NE(MicrotasksPolicy::kScoped, GetMicrotasksPolicy());
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
-  isolate->default_microtask_queue()->RunMicrotasks(isolate);
+  isolate->default_microtask_queue()->PerformCheckpoint(this);
 }
 
 void Isolate::EnqueueMicrotask(Local<Function> v8_function) {

deps/v8/src/d8/d8.cc

@@ -1066,7 +1066,7 @@ bool Shell::ExecuteModule(Isolate* isolate, const char* file_name) {
     // able to just busy loop waiting for execution to finish.
     Local<Promise> result_promise(Local<Promise>::Cast(result));
     while (result_promise->State() == Promise::kPending) {
-      isolate->RunMicrotasks();
+      isolate->PerformMicrotaskCheckpoint();
     }
 
     if (result_promise->State() == Promise::kRejected) {
@@ -3297,7 +3297,6 @@ bool ProcessMessages(
     while (v8::platform::PumpMessageLoop(g_default_platform, isolate,
                                          behavior())) {
       MicrotasksScope::PerformCheckpoint(isolate);
-      isolate->ClearKeptObjects();
     }
     if (g_default_platform->IdleTasksEnabled(isolate)) {
       v8::platform::RunIdleTasks(g_default_platform, isolate,

deps/v8/src/execution/microtask-queue.cc

@@ -115,6 +115,7 @@ void MicrotaskQueue::PerformCheckpoint(v8::Isolate* v8_isolate) {
       !HasMicrotasksSuppressions()) {
     Isolate* isolate = reinterpret_cast<Isolate*>(v8_isolate);
     RunMicrotasks(isolate);
+    isolate->ClearKeptObjects();
   }
 }
 

deps/v8/test/cctest/test-api.cc

@@ -19911,7 +19911,7 @@ TEST(RunMicrotasksIgnoresThrownExceptionsFromApi) {
     CHECK(!isolate->IsExecutionTerminating());
     isolate->EnqueueMicrotask(ThrowExceptionMicrotask);
     isolate->EnqueueMicrotask(IncrementCounterMicrotask);
-    isolate->RunMicrotasks();
+    isolate->PerformMicrotaskCheckpoint();
     CHECK_EQ(1, microtask_callback_count);
     CHECK(!try_catch.HasCaught());
   }
@@ -19953,7 +19953,7 @@ TEST(SetAutorunMicrotasks) {
   CHECK_EQ(0, CompileRun("ext2Calls")->Int32Value(env.local()).FromJust());
   CHECK_EQ(1u, microtasks_completed_callback_count);
 
-  env->GetIsolate()->RunMicrotasks();
+  env->GetIsolate()->PerformMicrotaskCheckpoint();
   CHECK_EQ(2, CompileRun("ext1Calls")->Int32Value(env.local()).FromJust());
   CHECK_EQ(1, CompileRun("ext2Calls")->Int32Value(env.local()).FromJust());
   CHECK_EQ(2u, microtasks_completed_callback_count);
@@ -19965,7 +19965,7 @@ TEST(SetAutorunMicrotasks) {
   CHECK_EQ(1, CompileRun("ext2Calls")->Int32Value(env.local()).FromJust());
   CHECK_EQ(2u, microtasks_completed_callback_count);
 
-  env->GetIsolate()->RunMicrotasks();
+  env->GetIsolate()->PerformMicrotaskCheckpoint();
   CHECK_EQ(2, CompileRun("ext1Calls")->Int32Value(env.local()).FromJust());
   CHECK_EQ(2, CompileRun("ext2Calls")->Int32Value(env.local()).FromJust());
   CHECK_EQ(3u, microtasks_completed_callback_count);
@@ -20015,7 +20015,7 @@ TEST(RunMicrotasksWithoutEnteringContext) {
     isolate->EnqueueMicrotask(
         Function::New(context, MicrotaskOne).ToLocalChecked());
   }
-  isolate->RunMicrotasks();
+  isolate->PerformMicrotaskCheckpoint();
   {
     Context::Scope context_scope(context);
     CHECK_EQ(1, CompileRun("ext1Calls")->Int32Value(context).FromJust());
@@ -20039,7 +20039,7 @@ static void Regress808911_CurrentContextWrapper(
   CHECK(isolate->GetCurrentContext() !=
         isolate->GetEnteredOrMicrotaskContext());
   isolate->EnqueueMicrotask(Regress808911_MicrotaskCallback, isolate);
-  isolate->RunMicrotasks();
+  isolate->PerformMicrotaskCheckpoint();
 }
 
 THREADED_TEST(Regress808911) {
@@ -22507,7 +22507,7 @@ TEST(PromiseThen) {
                   .ToLocalChecked()
                   ->Int32Value(context.local())
                   .FromJust());
-  isolate->RunMicrotasks();
+  isolate->PerformMicrotaskCheckpoint();
   CHECK_EQ(1, global->Get(context.local(), v8_str("x1"))
                   .ToLocalChecked()
                   ->Int32Value(context.local())
@@ -22533,7 +22533,7 @@ TEST(PromiseThen) {
                   .ToLocalChecked()
                   ->Int32Value(context.local())
                   .FromJust());
-  isolate->RunMicrotasks();
+  isolate->PerformMicrotaskCheckpoint();
   CHECK_EQ(0, global->Get(context.local(), v8_str("x1"))
                   .ToLocalChecked()
                   ->Int32Value(context.local())
@@ -22553,7 +22553,7 @@ TEST(PromiseThen) {
                   .ToLocalChecked()
                   ->Int32Value(context.local())
                   .FromJust());
-  isolate->RunMicrotasks();
+  isolate->PerformMicrotaskCheckpoint();
   CHECK_EQ(3, global->Get(context.local(), v8_str("x1"))
                   .ToLocalChecked()
                   ->Int32Value(context.local())
@@ -22602,7 +22602,7 @@ TEST(PromiseThen2) {
                   .ToLocalChecked()
                   ->Int32Value(context.local())
                   .FromJust());
-  isolate->RunMicrotasks();
+  isolate->PerformMicrotaskCheckpoint();
   CHECK_EQ(1, global->Get(context.local(), v8_str("x1"))
                   .ToLocalChecked()
                   ->Int32Value(context.local())
@@ -22613,7 +22613,7 @@ TEST(PromiseThen2) {
                   .FromJust());
 
   Local<v8::Promise> b = a->Then(context.local(), f3, f2).ToLocalChecked();
-  isolate->RunMicrotasks();
+  isolate->PerformMicrotaskCheckpoint();
   CHECK_EQ(1, global->Get(context.local(), v8_str("x1"))
                   .ToLocalChecked()
                   ->Int32Value(context.local())
@@ -22624,7 +22624,7 @@ TEST(PromiseThen2) {
                   .FromJust());
 
   Local<v8::Promise> c = b->Then(context.local(), f1, f2).ToLocalChecked();
-  isolate->RunMicrotasks();
+  isolate->PerformMicrotaskCheckpoint();
   CHECK_EQ(1, global->Get(context.local(), v8_str("x1"))
                   .ToLocalChecked()
                   ->Int32Value(context.local())
@@ -22635,7 +22635,7 @@ TEST(PromiseThen2) {
                     .FromJust());
 
   v8::Local<v8::Promise> d = c->Then(context.local(), f1, f2).ToLocalChecked();
-  isolate->RunMicrotasks();
+  isolate->PerformMicrotaskCheckpoint();
   CHECK_EQ(103, global->Get(context.local(), v8_str("x1"))
                     .ToLocalChecked()
                     ->Int32Value(context.local())
@@ -22646,7 +22646,7 @@ TEST(PromiseThen2) {
                     .FromJust());
 
   v8::Local<v8::Promise> e = d->Then(context.local(), f3, f2).ToLocalChecked();
-  isolate->RunMicrotasks();
+  isolate->PerformMicrotaskCheckpoint();
   CHECK_EQ(103, global->Get(context.local(), v8_str("x1"))
                     .ToLocalChecked()
                     ->Int32Value(context.local())
@@ -22657,7 +22657,7 @@ TEST(PromiseThen2) {
                     .FromJust());
 
   v8::Local<v8::Promise> f = e->Then(context.local(), f1, f3).ToLocalChecked();
-  isolate->RunMicrotasks();
+  isolate->PerformMicrotaskCheckpoint();
   CHECK_EQ(103, global->Get(context.local(), v8_str("x1"))
                     .ToLocalChecked()
                     ->Int32Value(context.local())
@@ -22668,7 +22668,7 @@ TEST(PromiseThen2) {
                     .FromJust());
 
   f->Then(context.local(), f1, f2).ToLocalChecked();
-  isolate->RunMicrotasks();
+  isolate->PerformMicrotaskCheckpoint();
   CHECK_EQ(103, global->Get(context.local(), v8_str("x1"))
                     .ToLocalChecked()
                     ->Int32Value(context.local())
@@ -25735,7 +25735,7 @@ TEST(DynamicImport) {
   i::MaybeHandle<i::JSPromise> maybe_promise =
       i_isolate->RunHostImportModuleDynamicallyCallback(referrer, specifier);
   i::Handle<i::JSPromise> promise = maybe_promise.ToHandleChecked();
-  isolate->RunMicrotasks();
+  isolate->PerformMicrotaskCheckpoint();
   CHECK(result->Equals(i::String::cast(promise->result())));
 }
 
@@ -26435,7 +26435,7 @@ TEST(MicrotaskContextShouldBeNativeContext) {
       "  await 42;"
       "})().then(callback);}");
 
-  isolate->RunMicrotasks();
+  isolate->PerformMicrotaskCheckpoint();
 }
 
 TEST(PreviewSetKeysIteratorEntriesWithDeleted) {

deps/v8/test/cctest/test-js-weak-refs.cc

@@ -158,6 +158,17 @@ void VerifyWeakCellKeyChain(Isolate* isolate, SimpleNumberDictionary key_map,
   va_end(args);
 }
 
+Handle<JSWeakRef> MakeWeakRefAndKeepDuringJob(Isolate* isolate) {
+  HandleScope inner_scope(isolate);
+
+  Handle<JSObject> js_object =
+      isolate->factory()->NewJSObject(isolate->object_function());
+  Handle<JSWeakRef> inner_weak_ref = ConstructJSWeakRef(js_object, isolate);
+  isolate->heap()->KeepDuringJob(js_object);
+
+  return inner_scope.CloseAndEscape(inner_weak_ref);
+}
+
 }  // namespace
 
 TEST(TestRegister) {
@@ -715,30 +726,35 @@ TEST(TestJSWeakRefKeepDuringJob) {
   LocalContext context;
 
   Isolate* isolate = CcTest::i_isolate();
-  Heap* heap = isolate->heap();
   HandleScope outer_scope(isolate);
-  Handle<JSWeakRef> weak_ref;
-  {
-    HandleScope inner_scope(isolate);
-
-    Handle<JSObject> js_object =
-        isolate->factory()->NewJSObject(isolate->object_function());
-    Handle<JSWeakRef> inner_weak_ref = ConstructJSWeakRef(js_object, isolate);
-    heap->KeepDuringJob(js_object);
+  Handle<JSWeakRef> weak_ref = MakeWeakRefAndKeepDuringJob(isolate);
+  CHECK(!weak_ref->target().IsUndefined(isolate));
+  CcTest::CollectAllGarbage();
+  CHECK(!weak_ref->target().IsUndefined(isolate));
 
-    weak_ref = inner_scope.CloseAndEscape(inner_weak_ref);
-  }
+  // Clears the KeepDuringJob set.
+  context->GetIsolate()->ClearKeptObjects();
+  CcTest::CollectAllGarbage();
+  CHECK(weak_ref->target().IsUndefined(isolate));
 
+  weak_ref = MakeWeakRefAndKeepDuringJob(isolate);
+  CHECK(!weak_ref->target().IsUndefined(isolate));
+  CcTest::CollectAllGarbage();
   CHECK(!weak_ref->target().IsUndefined(isolate));
 
+  // ClearKeptObjects should be called by PerformMicrotasksCheckpoint.
+  CcTest::isolate()->PerformMicrotaskCheckpoint();
   CcTest::CollectAllGarbage();
+  CHECK(weak_ref->target().IsUndefined(isolate));
 
+  weak_ref = MakeWeakRefAndKeepDuringJob(isolate);
   CHECK(!weak_ref->target().IsUndefined(isolate));
-
-  // Clears the KeepDuringJob set.
-  context->GetIsolate()->ClearKeptObjects();
   CcTest::CollectAllGarbage();
+  CHECK(!weak_ref->target().IsUndefined(isolate));
 
+  // ClearKeptObjects should be called by MicrotasksScope::PerformCheckpoint.
+  v8::MicrotasksScope::PerformCheckpoint(CcTest::isolate());
+  CcTest::CollectAllGarbage();
   CHECK(weak_ref->target().IsUndefined(isolate));
 }
 
@@ -754,17 +770,7 @@ TEST(TestJSWeakRefKeepDuringJobIncrementalMarking) {
   Isolate* isolate = CcTest::i_isolate();
   Heap* heap = isolate->heap();
   HandleScope outer_scope(isolate);
-  Handle<JSWeakRef> weak_ref;
-  {
-    HandleScope inner_scope(isolate);
-
-    Handle<JSObject> js_object =
-        isolate->factory()->NewJSObject(isolate->object_function());
-    Handle<JSWeakRef> inner_weak_ref = ConstructJSWeakRef(js_object, isolate);
-    heap->KeepDuringJob(js_object);
-
-    weak_ref = inner_scope.CloseAndEscape(inner_weak_ref);
-  }
+  Handle<JSWeakRef> weak_ref = MakeWeakRefAndKeepDuringJob(isolate);
 
   CHECK(!weak_ref->target().IsUndefined(isolate));
 

deps/v8/test/cctest/test-modules.cc

@@ -830,7 +830,7 @@ TEST(ModuleEvaluationTopLevelAwaitDynamicImport) {
     CHECK_EQ(promise->State(), v8::Promise::kPending);
     CHECK(!try_catch.HasCaught());
 
-    isolate->RunMicrotasks();
+    isolate->PerformMicrotaskCheckpoint();
     CHECK_EQ(promise->State(), v8::Promise::kFulfilled);
   }
   i::FLAG_harmony_top_level_await = previous_top_level_await_flag_value;
@@ -874,7 +874,7 @@ TEST(ModuleEvaluationTopLevelAwaitDynamicImportError) {
     CHECK_EQ(promise->State(), v8::Promise::kPending);
     CHECK(!try_catch.HasCaught());
 
-    isolate->RunMicrotasks();
+    isolate->PerformMicrotaskCheckpoint();
     CHECK_EQ(Module::kErrored, module->GetStatus());
     CHECK_EQ(promise->State(), v8::Promise::kRejected);
     CHECK(promise->Result()->StrictEquals(v8_str("boom")));