heap: Disallow allocation in first round of weak callbacks

Weak callbacks should not trigger recursive GCs during first round callbacks.
Any non-trivial work is supposed to be enqueued in the second round of
callbacks.

Bug: chromium:843903
Change-Id: Ieba58f31bab54c95b7d4027d3e16ee2d765438e7
Reviewed-on: https://chromium-review.googlesource.com/c/1340285
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57581}

src/global-handles.cc

@@ -834,11 +834,8 @@ void GlobalHandles::UpdateListOfNewSpaceNodes() {
   new_space_nodes_.shrink_to_fit();
 }
 
-
-int GlobalHandles::DispatchPendingPhantomCallbacks(
-    bool synchronous_second_pass) {
+int GlobalHandles::InvokeFirstPassWeakCallbacks() {
   int freed_nodes = 0;
-  // Protect against callback modifying pending_phantom_callbacks_.
   std::vector<PendingPhantomCallback> pending_phantom_callbacks;
   pending_phantom_callbacks.swap(pending_phantom_callbacks_);
   {
@@ -851,6 +848,11 @@ int GlobalHandles::DispatchPendingPhantomCallbacks(
       freed_nodes++;
     }
   }
+  return freed_nodes;
+}
+
+void GlobalHandles::InvokeOrScheduleSecondPassPhantomCallbacks(
+    bool synchronous_second_pass) {
   if (!second_pass_callbacks_.empty()) {
     if (FLAG_optimize_for_size || FLAG_predictable || synchronous_second_pass) {
       isolate()->heap()->CallGCPrologueCallbacks(
@@ -866,10 +868,8 @@ int GlobalHandles::DispatchPendingPhantomCallbacks(
           isolate(), [this] { InvokeSecondPassPhantomCallbacksFromTask(); }));
     }
   }
-  return freed_nodes;
 }
 
-
 void GlobalHandles::PendingPhantomCallback::Invoke(Isolate* isolate) {
   Data::Callback* callback_addr = nullptr;
   if (node_ != nullptr) {
@@ -893,7 +893,6 @@ void GlobalHandles::PendingPhantomCallback::Invoke(Isolate* isolate) {
   }
 }
 
-
 int GlobalHandles::PostGarbageCollectionProcessing(
     GarbageCollector collector, const v8::GCCallbackFlags gc_callback_flags) {
   // Process weak global handle callbacks. This must be done after the
@@ -907,7 +906,7 @@ int GlobalHandles::PostGarbageCollectionProcessing(
       (gc_callback_flags &
        (kGCCallbackFlagForced | kGCCallbackFlagCollectAllAvailableGarbage |
         kGCCallbackFlagSynchronousPhantomCallbackProcessing)) != 0;
-  freed_nodes += DispatchPendingPhantomCallbacks(synchronous_second_pass);
+  InvokeOrScheduleSecondPassPhantomCallbacks(synchronous_second_pass);
   if (initial_post_gc_processing_count != post_gc_processing_count_) {
     // If the callbacks caused a nested GC, then return.  See comment in
     // PostScavengeProcessing.

src/global-handles.h

@@ -108,6 +108,8 @@ class GlobalHandles {
   // Tells whether global handle is weak.
   static bool IsWeak(Address* location);
 
+  int InvokeFirstPassWeakCallbacks();
+
   // Process pending weak handles.
   // Returns the number of freed nodes.
   int PostGarbageCollectionProcessing(
@@ -193,7 +195,7 @@ class GlobalHandles {
   void InvokeSecondPassPhantomCallbacksFromTask();
   int PostScavengeProcessing(int initial_post_gc_processing_count);
   int PostMarkSweepProcessing(int initial_post_gc_processing_count);
-  int DispatchPendingPhantomCallbacks(bool synchronous_second_pass);
+  void InvokeOrScheduleSecondPassPhantomCallbacks(bool synchronous_second_pass);
   void UpdateListOfNewSpaceNodes();
   void ApplyPersistentHandleVisitor(v8::PersistentHandleVisitor* visitor,
                                     Node* node);

src/heap/heap.cc

@@ -1700,15 +1700,22 @@ bool Heap::PerformGarbageCollection(
 
   isolate_->counters()->objs_since_last_young()->Set(0);
 
-  gc_post_processing_depth_++;
   {
-    AllowHeapAllocation allow_allocation;
     TRACE_GC(tracer(), GCTracer::Scope::HEAP_EXTERNAL_WEAK_GLOBAL_HANDLES);
+    // First round weak callbacks are not supposed to allocate and trigger
+    // nested GCs.
     freed_global_handles =
-        isolate_->global_handles()->PostGarbageCollectionProcessing(
-            collector, gc_callback_flags);
+        isolate_->global_handles()->InvokeFirstPassWeakCallbacks();
+
+    gc_post_processing_depth_++;
+    {
+      AllowHeapAllocation allow_allocation;
+      freed_global_handles +=
+          isolate_->global_handles()->PostGarbageCollectionProcessing(
+              collector, gc_callback_flags);
+    }
+    gc_post_processing_depth_--;
   }
-  gc_post_processing_depth_--;
 
   isolate_->eternal_handles()->PostGarbageCollectionProcessing();