[weakrefs] Schedule FinalizationGroup cleanup tasks from within V8

Deprecate the following explicit FinalizationGroup APIs in favor of
automatic handling of FinalizationGroup cleanup callbacks:
  - v8::Isolate::SetHostCleanupFinalizationGroupCallback
  - v8::FinaliationGroup::Cleanup

If no HostCleanupFinalizationGroupCallback is set, then
FinalizationGroup cleanup callbacks are automatically scheduled by V8
itself as non-nestable foreground tasks.

When a Context being disposed, all FinalizationGroups that are
associated with it are removed from the dirty list, cancelling
scheduled cleanup.

Bug: v8:8179
Change-Id: Ic09313a11dd00af36d1f698250b3d735155f45e8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1986392
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66184}

BUILD.gn

@@ -2299,6 +2299,8 @@ v8_source_set("v8_base_without_compiler") {
     "src/heap/factory-inl.h",
     "src/heap/factory.cc",
     "src/heap/factory.h",
+    "src/heap/finalization-group-cleanup-task.cc",
+    "src/heap/finalization-group-cleanup-task.h",
     "src/heap/gc-idle-time-handler.cc",
     "src/heap/gc-idle-time-handler.h",
     "src/heap/gc-tracer.cc",

include/v8.h

@@ -5912,6 +5912,9 @@ class V8_EXPORT FinalizationGroup : public Object {
    * occurred. Otherwise the result is |true| if the cleanup callback
    * was called successfully. The result is never |false|.
    */
+  V8_DEPRECATED(
+      "FinalizationGroup cleanup is automatic if "
+      "HostCleanupFinalizationGroupCallback is not set")
   static V8_WARN_UNUSED_RESULT Maybe<bool> Cleanup(
       Local<FinalizationGroup> finalization_group);
 };
@@ -8481,6 +8484,9 @@ class V8_EXPORT Isolate {
    * are ready to be cleaned up and require FinalizationGroup::Cleanup()
    * to be called in a future task.
    */
+  V8_DEPRECATED(
+      "FinalizationGroup cleanup is automatic if "
+      "HostCleanupFinalizationGroupCallback is not set")
   void SetHostCleanupFinalizationGroupCallback(
       HostCleanupFinalizationGroupCallback callback);
 
@@ -9085,10 +9091,10 @@ class V8_EXPORT Isolate {
   void LowMemoryNotification();
 
   /**
-   * Optional notification that a context has been disposed. V8 uses
-   * these notifications to guide the GC heuristic. Returns the number
-   * of context disposals - including this one - since the last time
-   * V8 had a chance to clean up.
+   * Optional notification that a context has been disposed. V8 uses these
+   * notifications to guide the GC heuristic and cancel FinalizationGroup
+   * cleanup tasks. Returns the number of context disposals - including this one
+   * - since the last time V8 had a chance to clean up.
    *
    * The optional parameter |dependant_context| specifies whether the disposed
    * context was depending on state from other contexts or not.

src/api/api.cc

@@ -11033,6 +11033,26 @@ void InvokeFunctionCallback(const v8::FunctionCallbackInfo<v8::Value>& info,
   callback(info);
 }
 
+void InvokeFinalizationGroupCleanupFromTask(
+    Handle<Context> context, Handle<JSFinalizationGroup> finalization_group,
+    Handle<Object> callback) {
+  Isolate* isolate = finalization_group->native_context().GetIsolate();
+  RuntimeCallTimerScope timer(
+      isolate, RuntimeCallCounterId::kFinalizationGroupCleanupFromTask);
+  // Do not use ENTER_V8 because this is always called from a running
+  // FinalizationGroupCleanupTask within V8 and we should not log it as an API
+  // call. This method is implemented here to avoid duplication of the exception
+  // handling and microtask running logic in CallDepthScope.
+  if (IsExecutionTerminatingCheck(isolate)) return;
+  Local<v8::Context> api_context = Utils::ToLocal(context);
+  CallDepthScope<true> call_depth_scope(isolate, api_context);
+  VMState<OTHER> state(isolate);
+  if (JSFinalizationGroup::Cleanup(isolate, finalization_group, callback)
+          .IsNothing()) {
+    call_depth_scope.Escape();
+  }
+}
+
 // Undefine macros for jumbo build.
 #undef LOG_API
 #undef ENTER_V8_DO_NOT_USE

src/api/api.h

@@ -26,6 +26,7 @@ namespace v8 {
 
 namespace internal {
 class JSArrayBufferView;
+class JSFinalizationGroup;
 }  // namespace internal
 
 namespace debug {
@@ -561,6 +562,10 @@ void InvokeAccessorGetterCallback(
 void InvokeFunctionCallback(const v8::FunctionCallbackInfo<v8::Value>& info,
                             v8::FunctionCallback callback);
 
+void InvokeFinalizationGroupCleanupFromTask(
+    Handle<Context> context, Handle<JSFinalizationGroup> finalization_group,
+    Handle<Object> callback);
+
 }  // namespace internal
 }  // namespace v8
 

src/builtins/builtins-weak-refs.cc

@@ -148,9 +148,8 @@ BUILTIN(FinalizationGroupCleanupSome) {
     callback = callback_obj;
   }
 
-  // Don't do set_scheduled_for_cleanup(false); we still have the microtask
-  // scheduled and don't want to schedule another one in case the user never
-  // executes microtasks.
+  // Don't invalidate the cleanup task id; we still have the task scheduled and
+  // don't want to schedule another one.
   if (JSFinalizationGroup::Cleanup(isolate, finalization_group, callback)
           .IsNothing()) {
     DCHECK(isolate->has_pending_exception());

src/d8/d8.cc

@@ -916,16 +916,6 @@ MaybeLocal<Promise> Shell::HostImportModuleDynamically(
   return MaybeLocal<Promise>();
 }
 
-void Shell::HostCleanupFinalizationGroup(Local<Context> context,
-                                         Local<FinalizationGroup> fg) {
-  Isolate* isolate = context->GetIsolate();
-  PerIsolateData::Get(isolate)->HostCleanupFinalizationGroup(fg);
-}
-
-void PerIsolateData::HostCleanupFinalizationGroup(Local<FinalizationGroup> fg) {
-  cleanup_finalization_groups_.emplace(isolate_, fg);
-}
-
 void Shell::HostInitializeImportMetaObject(Local<Context> context,
                                            Local<Module> module,
                                            Local<Object> meta) {
@@ -1123,15 +1113,6 @@ MaybeLocal<Context> PerIsolateData::GetTimeoutContext() {
   return result;
 }
 
-MaybeLocal<FinalizationGroup> PerIsolateData::GetCleanupFinalizationGroup() {
-  if (cleanup_finalization_groups_.empty())
-    return MaybeLocal<FinalizationGroup>();
-  Local<FinalizationGroup> result =
-      cleanup_finalization_groups_.front().Get(isolate_);
-  cleanup_finalization_groups_.pop();
-  return result;
-}
-
 PerIsolateData::RealmScope::RealmScope(PerIsolateData* data) : data_(data) {
   data_->realm_count_ = 1;
   data_->realm_current_ = 0;
@@ -1281,8 +1262,11 @@ void Shell::DisposeRealm(const v8::FunctionCallbackInfo<v8::Value>& args,
                          int index) {
   Isolate* isolate = args.GetIsolate();
   PerIsolateData* data = PerIsolateData::Get(isolate);
-  DisposeModuleEmbedderData(data->realms_[index].Get(isolate));
+  Local<Context> context = data->realms_[index].Get(isolate);
+  DisposeModuleEmbedderData(context);
   data->realms_[index].Reset();
+  // ContextDisposedNotification expects the disposed context to be entered.
+  v8::Context::Scope scope(context);
   isolate->ContextDisposedNotification();
   isolate->IdleNotificationDeadline(g_platform->MonotonicallyIncreasingTime());
 }
@@ -2742,8 +2726,6 @@ void SourceGroup::ExecuteInThread() {
   Isolate::CreateParams create_params;
   create_params.array_buffer_allocator = Shell::array_buffer_allocator;
   Isolate* isolate = Isolate::New(create_params);
-  isolate->SetHostCleanupFinalizationGroupCallback(
-      Shell::HostCleanupFinalizationGroup);
   isolate->SetHostImportModuleDynamicallyCallback(
       Shell::HostImportModuleDynamically);
   isolate->SetHostInitializeImportMetaObjectCallback(
@@ -2889,8 +2871,6 @@ void Worker::ExecuteInThread() {
   Isolate::CreateParams create_params;
   create_params.array_buffer_allocator = Shell::array_buffer_allocator;
   Isolate* isolate = Isolate::New(create_params);
-  isolate->SetHostCleanupFinalizationGroupCallback(
-      Shell::HostCleanupFinalizationGroup);
   isolate->SetHostImportModuleDynamicallyCallback(
       Shell::HostImportModuleDynamically);
   isolate->SetHostInitializeImportMetaObjectCallback(
@@ -3272,21 +3252,6 @@ bool RunSetTimeoutCallback(Isolate* isolate, bool* did_run) {
   return true;
 }
 
-bool RunCleanupFinalizationGroupCallback(Isolate* isolate, bool* did_run) {
-  PerIsolateData* data = PerIsolateData::Get(isolate);
-  HandleScope handle_scope(isolate);
-  while (true) {
-    Local<FinalizationGroup> fg;
-    if (!data->GetCleanupFinalizationGroup().ToLocal(&fg)) return true;
-    *did_run = true;
-    TryCatch try_catch(isolate);
-    try_catch.SetVerbose(true);
-    if (FinalizationGroup::Cleanup(fg).IsNothing()) {
-      return false;
-    }
-  }
-}
-
 bool ProcessMessages(
     Isolate* isolate,
     const std::function<platform::MessageLoopBehavior()>& behavior) {
@@ -3303,17 +3268,12 @@ bool ProcessMessages(
       v8::platform::RunIdleTasks(g_default_platform, isolate,
                                  50.0 / base::Time::kMillisecondsPerSecond);
     }
-    bool ran_finalization_callback = false;
-    if (!RunCleanupFinalizationGroupCallback(isolate,
-                                             &ran_finalization_callback)) {
-      return false;
-    }
     bool ran_set_timeout = false;
     if (!RunSetTimeoutCallback(isolate, &ran_set_timeout)) {
       return false;
     }
 
-    if (!ran_set_timeout && !ran_finalization_callback) return true;
+    if (!ran_set_timeout) return true;
   }
   return true;
 }
@@ -3768,8 +3728,6 @@ int Shell::Main(int argc, char* argv[]) {
   }
 
   Isolate* isolate = Isolate::New(create_params);
-  isolate->SetHostCleanupFinalizationGroupCallback(
-      Shell::HostCleanupFinalizationGroup);
   isolate->SetHostImportModuleDynamicallyCallback(
       Shell::HostImportModuleDynamically);
   isolate->SetHostInitializeImportMetaObjectCallback(
@@ -3832,8 +3790,6 @@ int Shell::Main(int argc, char* argv[]) {
       i::FLAG_hash_seed ^= 1337;  // Use a different hash seed.
       Isolate* isolate2 = Isolate::New(create_params);
       i::FLAG_hash_seed ^= 1337;  // Restore old hash seed.
-      isolate2->SetHostCleanupFinalizationGroupCallback(
-          Shell::HostCleanupFinalizationGroup);
       isolate2->SetHostImportModuleDynamicallyCallback(
           Shell::HostImportModuleDynamically);
       isolate2->SetHostInitializeImportMetaObjectCallback(

src/d8/d8.h

@@ -226,8 +226,6 @@ class PerIsolateData {
     PerIsolateData* data_;
   };
 
-  inline void HostCleanupFinalizationGroup(Local<FinalizationGroup> fg);
-  inline MaybeLocal<FinalizationGroup> GetCleanupFinalizationGroup();
   inline void SetTimeout(Local<Function> callback, Local<Context> context);
   inline MaybeLocal<Function> GetTimeoutCallback();
   inline MaybeLocal<Context> GetTimeoutContext();
@@ -245,7 +243,6 @@ class PerIsolateData {
   Global<Value> realm_shared_;
   std::queue<Global<Function>> set_timeout_callbacks_;
   std::queue<Global<Context>> set_timeout_contexts_;
-  std::queue<Global<FinalizationGroup>> cleanup_finalization_groups_;
   AsyncHooks* async_hooks_wrapper_;
 
   int RealmIndexOrThrow(const v8::FunctionCallbackInfo<v8::Value>& args,
@@ -423,8 +420,6 @@ class Shell : public i::AllStatic {
   static void SetUMask(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void MakeDirectory(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void RemoveDirectory(const v8::FunctionCallbackInfo<v8::Value>& args);
-  static void HostCleanupFinalizationGroup(Local<Context> context,
-                                           Local<FinalizationGroup> fg);
   static MaybeLocal<Promise> HostImportModuleDynamically(
       Local<Context> context, Local<ScriptOrModule> referrer,
       Local<String> specifier);

src/execution/isolate.h

@@ -1404,6 +1404,10 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   void ClearKeptObjects();
   void SetHostCleanupFinalizationGroupCallback(
       HostCleanupFinalizationGroupCallback callback);
+  HostCleanupFinalizationGroupCallback
+  host_cleanup_finalization_group_callback() const {
+    return host_cleanup_finalization_group_callback_;
+  }
   void RunHostCleanupFinalizationGroupCallback(Handle<JSFinalizationGroup> fg);
 
   void SetHostImportModuleDynamicallyCallback(

src/heap/finalization-group-cleanup-task.cc

@@ -0,0 +1,71 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/heap/finalization-group-cleanup-task.h"
+
+#include "src/execution/frames.h"
+#include "src/execution/interrupts-scope.h"
+#include "src/execution/stack-guard.h"
+#include "src/execution/v8threads.h"
+#include "src/heap/heap-inl.h"
+#include "src/objects/js-weak-refs-inl.h"
+#include "src/tracing/trace-event.h"
+
+namespace v8 {
+namespace internal {
+
+FinalizationGroupCleanupTask::FinalizationGroupCleanupTask(Heap* heap)
+    : CancelableTask(heap->isolate()), heap_(heap) {}
+
+void FinalizationGroupCleanupTask::SlowAssertNoActiveJavaScript() {
+#ifdef ENABLE_SLOW_DCHECKS
+  class NoActiveJavaScript : public ThreadVisitor {
+   public:
+    void VisitThread(Isolate* isolate, ThreadLocalTop* top) override {
+      for (StackFrameIterator it(isolate, top); !it.done(); it.Advance()) {
+        DCHECK(!it.frame()->is_java_script());
+      }
+    }
+  };
+  NoActiveJavaScript no_active_js_visitor;
+  Isolate* isolate = heap_->isolate();
+  no_active_js_visitor.VisitThread(isolate, isolate->thread_local_top());
+  isolate->thread_manager()->IterateArchivedThreads(&no_active_js_visitor);
+#endif  // ENABLE_SLOW_DCHECKS
+}
+
+void FinalizationGroupCleanupTask::RunInternal() {
+  Isolate* isolate = heap_->isolate();
+  DCHECK(!isolate->host_cleanup_finalization_group_callback());
+  SlowAssertNoActiveJavaScript();
+
+  TRACE_EVENT_CALL_STATS_SCOPED(isolate, "v8",
+                                "V8.FinalizationGroupCleanupTask");
+
+  HandleScope handle_scope(isolate);
+  Handle<JSFinalizationGroup> finalization_group =
+      heap_->TakeOneDirtyJSFinalizationGroup().ToHandleChecked();
+  finalization_group->set_scheduled_for_cleanup(false);
+
+  // Since FinalizationGroup cleanup callbacks are scheduled by V8, enter the
+  // FinalizationGroup's context.
+  Handle<Context> context(Context::cast(finalization_group->native_context()),
+                          isolate);
+  Handle<Object> callback(finalization_group->cleanup(), isolate);
+  v8::Context::Scope context_scope(v8::Utils::ToLocal(context));
+  v8::TryCatch catcher(reinterpret_cast<v8::Isolate*>(isolate));
+  catcher.SetVerbose(true);
+
+  // Ignore the result because exceptions are reported via the message
+  // handler. This is ensured by the verbose TryCatch.
+  InvokeFinalizationGroupCleanupFromTask(context, finalization_group, callback);
+
+  // Clear the task id and repost if there are remaining dirty
+  // FinalizationGroups.
+  heap_->ClearFinalizationGroupCleanupTaskId();
+  heap_->PostOrAbortFinalizationGroupCleanupTaskIfNeeded();
+}
+
+}  // namespace internal
+}  // namespace v8

src/heap/finalization-group-cleanup-task.h

@@ -0,0 +1,35 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_HEAP_FINALIZATION_GROUP_CLEANUP_TASK_H_
+#define V8_HEAP_FINALIZATION_GROUP_CLEANUP_TASK_H_
+
+#include "src/objects/js-weak-refs.h"
+#include "src/tasks/cancelable-task.h"
+
+namespace v8 {
+namespace internal {
+
+// The GC schedules a cleanup task when there the dirty FinalizationGroup list
+// is non-empty. The task itself schedules another cleanup task if there are
+// remaining dirty FinalizationGroups on the list.
+class FinalizationGroupCleanupTask : public CancelableTask {
+ public:
+  explicit FinalizationGroupCleanupTask(Heap* heap);
+  ~FinalizationGroupCleanupTask() override = default;
+
+ private:
+  FinalizationGroupCleanupTask(const FinalizationGroupCleanupTask&) = delete;
+  void operator=(const FinalizationGroupCleanupTask&) = delete;
+
+  void SlowAssertNoActiveJavaScript();
+  void RunInternal() override;
+
+  Heap* heap_;
+};
+
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_HEAP_FINALIZATION_GROUP_CLEANUP_TASK_H_

src/heap/heap-inl.h

@@ -616,6 +616,10 @@ void Heap::DecrementExternalBackingStoreBytes(ExternalBackingStoreType type,
   base::CheckedDecrement(&backing_store_bytes_, amount);
 }
 
+bool Heap::HasDirtyJSFinalizationGroups() {
+  return !dirty_js_finalization_groups().IsUndefined(isolate());
+}
+
 AlwaysAllocateScope::AlwaysAllocateScope(Heap* heap) : heap_(heap) {
   heap_->always_allocate_scope_count_++;
 }

src/heap/heap.cc

@@ -33,6 +33,7 @@
 #include "src/heap/combined-heap.h"
 #include "src/heap/concurrent-marking.h"
 #include "src/heap/embedder-tracing.h"
+#include "src/heap/finalization-group-cleanup-task.h"
 #include "src/heap/gc-idle-time-handler.h"
 #include "src/heap/gc-tracer.h"
 #include "src/heap/heap-controller.h"
@@ -1197,17 +1198,11 @@ void Heap::GarbageCollectionEpilogue() {
     ReduceNewSpaceSize();
   }
 
-  if (FLAG_harmony_weak_refs) {
+  if (FLAG_harmony_weak_refs &&
+      isolate()->host_cleanup_finalization_group_callback()) {
     HandleScope handle_scope(isolate());
-    while (!isolate()->heap()->dirty_js_finalization_groups().IsUndefined(
-        isolate())) {
-      Handle<JSFinalizationGroup> finalization_group(
-          JSFinalizationGroup::cast(
-              isolate()->heap()->dirty_js_finalization_groups()),
-          isolate());
-      isolate()->heap()->set_dirty_js_finalization_groups(
-          finalization_group->next());
-      finalization_group->set_next(ReadOnlyRoots(isolate()).undefined_value());
+    Handle<JSFinalizationGroup> finalization_group;
+    while (TakeOneDirtyJSFinalizationGroup().ToHandle(&finalization_group)) {
       isolate()->RunHostCleanupFinalizationGroupCallback(finalization_group);
     }
   }
@@ -1662,6 +1657,9 @@ int Heap::NotifyContextDisposed(bool dependant_context) {
     memory_reducer_->NotifyPossibleGarbage(event);
   }
   isolate()->AbortConcurrentOptimization(BlockingBehavior::kDontBlock);
+  if (!isolate()->context().is_null()) {
+    RemoveDirtyFinalizationGroupsOnContext(isolate()->raw_native_context());
+  }
 
   number_of_disposed_maps_ = retained_maps().length();
   tracer()->AddContextDisposalTime(MonotonicallyIncreasingTimeInMs());
@@ -6019,11 +6017,35 @@ void Heap::SetInterpreterEntryTrampolineForProfiling(Code code) {
   set_interpreter_entry_trampoline_for_profiling(code);
 }
 
+void Heap::PostOrAbortFinalizationGroupCleanupTaskIfNeeded() {
+  DCHECK(!isolate()->host_cleanup_finalization_group_callback());
+
+  const bool has_dirty_finalization_groups = HasDirtyJSFinalizationGroups();
+  const bool is_cleanup_task_posted = finalization_group_cleanup_task_id_ !=
+                                      CancelableTaskManager::kInvalidTaskId;
+
+  if (!has_dirty_finalization_groups && is_cleanup_task_posted) {
+    // The task may have already ran, in which case it can't be aborted. The
+    // cleanup task can never be concurrently running since it is a
+    // non-nestable foreground task.
+    CancelableTaskManager* task_manager = isolate()->cancelable_task_manager();
+    CHECK(task_manager->TryAbort(finalization_group_cleanup_task_id_) !=
+          TryAbortResult::kTaskRunning);
+    ClearFinalizationGroupCleanupTaskId();
+  } else if (has_dirty_finalization_groups && !is_cleanup_task_posted) {
+    auto taskrunner = V8::GetCurrentPlatform()->GetForegroundTaskRunner(
+        reinterpret_cast<v8::Isolate*>(isolate()));
+    auto task = std::make_unique<FinalizationGroupCleanupTask>(this);
+    finalization_group_cleanup_task_id_ = task->id();
+    taskrunner->PostNonNestableTask(std::move(task));
+  }
+}
+
 void Heap::AddDirtyJSFinalizationGroup(
     JSFinalizationGroup finalization_group,
     std::function<void(HeapObject object, ObjectSlot slot, Object target)>
         gc_notify_updated_slot) {
-  DCHECK(dirty_js_finalization_groups().IsUndefined(isolate()) ||
+  DCHECK(!HasDirtyJSFinalizationGroups() ||
          dirty_js_finalization_groups().IsJSFinalizationGroup());
   DCHECK(finalization_group.next().IsUndefined(isolate()));
   DCHECK(!finalization_group.scheduled_for_cleanup());
@@ -6038,6 +6060,47 @@ void Heap::AddDirtyJSFinalizationGroup(
   // for the root pointing to the first JSFinalizationGroup.
 }
 
+MaybeHandle<JSFinalizationGroup> Heap::TakeOneDirtyJSFinalizationGroup() {
+  if (HasDirtyJSFinalizationGroups()) {
+    Handle<JSFinalizationGroup> finalization_group(
+        JSFinalizationGroup::cast(dirty_js_finalization_groups()), isolate());
+    set_dirty_js_finalization_groups(finalization_group->next());
+    finalization_group->set_next(ReadOnlyRoots(isolate()).undefined_value());
+    return finalization_group;
+  }
+  return {};
+}
+
+void Heap::RemoveDirtyFinalizationGroupsOnContext(NativeContext context) {
+  if (!FLAG_harmony_weak_refs) return;
+  if (isolate()->host_cleanup_finalization_group_callback()) return;
+
+  DisallowHeapAllocation no_gc;
+
+  // Remove all dirty FinalizationGroups for context.
+  Isolate* isolate = this->isolate();
+  Object prev = ReadOnlyRoots(isolate).undefined_value();
+  Object current = dirty_js_finalization_groups();
+  while (!current.IsUndefined(isolate)) {
+    JSFinalizationGroup finalization_group = JSFinalizationGroup::cast(current);
+    if (finalization_group.native_context() == context) {
+      if (prev.IsUndefined(isolate)) {
+        set_dirty_js_finalization_groups(finalization_group.next());
+      } else {
+        JSFinalizationGroup::cast(prev).set_next(finalization_group.next());
+      }
+      finalization_group.set_scheduled_for_cleanup(false);
+      current = finalization_group.next();
+      finalization_group.set_next(ReadOnlyRoots(isolate).undefined_value());
+    } else {
+      prev = current;
+      current = finalization_group.next();
+    }
+  }
+
+  PostOrAbortFinalizationGroupCleanupTaskIfNeeded();
+}
+
 void Heap::KeepDuringJob(Handle<JSReceiver> target) {
   DCHECK(FLAG_harmony_weak_refs);
   DCHECK(weak_refs_keep_during_job().IsUndefined() ||

src/heap/heap.h

@@ -30,6 +30,7 @@
 #include "src/objects/string-table.h"
 #include "src/objects/visitors.h"
 #include "src/roots/roots.h"
+#include "src/tasks/cancelable-task.h"
 #include "src/utils/allocation.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
 
@@ -797,12 +798,31 @@ class Heap {
   // See also: FLAG_interpreted_frames_native_stack.
   void SetInterpreterEntryTrampolineForProfiling(Code code);
 
-  // Add finalization_group into the dirty_js_finalization_groups list.
+  // Add finalization_group to the end of the dirty_js_finalization_groups list.
   void AddDirtyJSFinalizationGroup(
       JSFinalizationGroup finalization_group,
       std::function<void(HeapObject object, ObjectSlot slot, Object target)>
           gc_notify_updated_slot);
 
+  // Pop and return the head of the dirty_js_finalization_groups list.
+  MaybeHandle<JSFinalizationGroup> TakeOneDirtyJSFinalizationGroup();
+
+  // Called from Heap::NotifyContextDisposed to remove all FinalizationGroups
+  // with {context} from the dirty list when the context e.g. navigates away or
+  // is detached. If the dirty list is empty afterwards, the cleanup task is
+  // aborted if needed.
+  void RemoveDirtyFinalizationGroupsOnContext(NativeContext context);
+
+  inline bool HasDirtyJSFinalizationGroups();
+
+  void PostOrAbortFinalizationGroupCleanupTaskIfNeeded();
+
+  // Must only be called after a FinalizationGroupCleanupTask has run or is
+  // aborted.
+  void ClearFinalizationGroupCleanupTaskId() {
+    finalization_group_cleanup_task_id_ = CancelableTaskManager::kInvalidTaskId;
+  }
+
   V8_EXPORT_PRIVATE void KeepDuringJob(Handle<JSReceiver> target);
   void ClearKeptObjects();
 
@@ -2151,6 +2171,9 @@ class Heap {
 
   std::vector<HeapObjectAllocationTracker*> allocation_trackers_;
 
+  CancelableTaskManager::Id finalization_group_cleanup_task_id_ =
+      CancelableTaskManager::kInvalidTaskId;
+
   std::unique_ptr<third_party_heap::Heap> tp_heap_;
 
   // Classes in "heap" can be friends.

src/heap/mark-compact.cc

@@ -2534,6 +2534,9 @@ void MarkCompactCollector::ClearJSWeakRefs() {
       RecordSlot(weak_cell, slot, HeapObject::cast(*slot));
     }
   }
+  if (!isolate()->host_cleanup_finalization_group_callback()) {
+    heap()->PostOrAbortFinalizationGroupCleanupTaskIfNeeded();
+  }
 }
 
 void MarkCompactCollector::AbortWeakObjects() {

src/logging/counters.h

@@ -989,6 +989,7 @@ class RuntimeCallTimer final {
   V(DeoptimizeCode)                            \
   V(DeserializeContext)                        \
   V(DeserializeIsolate)                        \
+  V(FinalizationGroupCleanupFromTask)          \
   V(FunctionCallback)                          \
   V(FunctionLengthGetter)                      \
   V(FunctionPrototypeGetter)                   \

src/objects/js-weak-refs.h

@@ -6,7 +6,6 @@
 #define V8_OBJECTS_JS_WEAK_REFS_H_
 
 #include "src/objects/js-objects.h"
-#include "src/objects/microtask.h"
 
 // Has to be the last include (doesn't have include guards):
 #include "src/objects/object-macros.h"

src/tasks/cancelable-task.h

@@ -33,6 +33,8 @@ class V8_EXPORT_PRIVATE CancelableTaskManager {
  public:
   using Id = uint64_t;
 
+  static constexpr Id kInvalidTaskId = 0;
+
   CancelableTaskManager();
 
   ~CancelableTaskManager();
@@ -68,8 +70,6 @@ class V8_EXPORT_PRIVATE CancelableTaskManager {
   bool canceled() const { return canceled_; }
 
  private:
-  static constexpr Id kInvalidTaskId = 0;
-
   // Only called by {Cancelable} destructor. The task is done with executing,
   // but needs to be removed.
   void RemoveFinishedTask(Id id);

test/message/weakref-finalizationgroup-error.js

@@ -0,0 +1,26 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-weak-refs --expose-gc --noincremental-marking
+// Flags: --no-stress-opt
+
+// Since cleanup tasks are top-level tasks, errors thrown from them don't stop
+// future cleanup tasks from running.
+
+function callback(iter) {
+  [...iter];
+  throw new Error('callback');
+};
+
+const fg1 = new FinalizationGroup(callback);
+const fg2 = new FinalizationGroup(callback);
+
+(function() {
+let x = {};
+fg1.register(x, {});
+fg2.register(x, {});
+x = null;
+})();
+
+gc();

test/message/weakref-finalizationgroup-error.out

@@ -0,0 +1,12 @@
+*%(basename)s:{NUMBER}: Error: callback
+  throw new Error('callback');
+  ^
+Error: callback
+    at callback (*%(basename)s:{NUMBER}:{NUMBER})
+
+*%(basename)s:{NUMBER}: Error: callback
+  throw new Error('callback');
+  ^
+Error: callback
+    at callback (*%(basename)s:{NUMBER}:{NUMBER})
+

test/mjsunit/harmony/weakrefs/cleanup-on-detached-realm.js

@@ -9,15 +9,28 @@ let r = Realm.create();
 let FG = Realm.eval(r, "FinalizationGroup");
 Realm.detachGlobal(r);
 
+let fg_not_run = new FG(() => {
+  assertUnreachable();
+});
+(() => {
+  fg_not_run.register({});
+})();
+
+gc();
+
+// Disposing the realm cancels the already scheduled fg_not_run's finalizer.
+Realm.dispose(r);
+
 let fg = new FG(()=> {
   cleanedUp = true;
 });
 
+// FGs that are alive after disposal can still schedule tasks.
 (() => {
   let object = {};
   fg.register(object, {});
 
-  // object goes out of scope.
+  // object becomes unreachable.
 })();
 
 gc();