heap: Prohibit allocations in PreFinalizers

Replace ObjectResurrectionForbiddenScope with NoAllocationScope

Bug: 981414
Change-Id: Iced48387c20c431ee0918f4c239f6ef0b8de82a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1702368
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#681787}

third_party/blink/renderer/platform/heap/BlinkGCAPIReference.md

@@ -222,7 +222,7 @@ class Child : public Parent {
 ```
 
 *Notes*
-- Pre-finalizers are not allowed to resurrect objects, i.e., they are not allowed to relink dead objects into the object graph.
+- Pre-finalizers are not allowed to allocate new on-heap objects or resurrect objects (i.e., they are not allowed to relink dead objects into the object graph).
 - Pre-finalizers have some implications on the garbage collector's performance: the garbage-collector needs to iterate all registered pre-finalizers at every GC.
 Therefore, a pre-finalizer should be avoided unless it is really necessary.
 Especially, avoid defining a pre-finalizer in a class that can be allocated a lot.

third_party/blink/renderer/platform/heap/heap_test.cc

@@ -1427,29 +1427,6 @@ class FinalizationAllocator
   Persistent<IntWrapper>* wrapper_;
 };
 
-class PreFinalizationAllocator
-    : public GarbageCollectedFinalized<PreFinalizationAllocator> {
-  USING_PRE_FINALIZER(PreFinalizationAllocator, Dispose);
-
- public:
-  PreFinalizationAllocator(Persistent<IntWrapper>* wrapper)
-      : wrapper_(wrapper) {}
-
-  void Dispose() {
-    for (int i = 0; i < 10; ++i)
-      *wrapper_ = MakeGarbageCollected<IntWrapper>(42);
-    for (int i = 0; i < 512; ++i)
-      MakeGarbageCollected<OneKiloByteObject>();
-    for (int i = 0; i < 32; ++i)
-      MakeGarbageCollected<LargeHeapObject>();
-  }
-
-  void Trace(blink::Visitor* visitor) {}
-
- private:
-  Persistent<IntWrapper>* wrapper_;
-};
-
 class PreFinalizerBackingShrinkForbidden
     : public GarbageCollectedFinalized<PreFinalizerBackingShrinkForbidden> {
   USING_PRE_FINALIZER(PreFinalizerBackingShrinkForbidden, Dispose);
@@ -1499,6 +1476,9 @@ class PreFinalizerBackingShrinkForbidden
   HeapHashMap<int, Member<IntWrapper>> map_;
 };
 
+// Following 2 tests check for allocation failures. These failures happen
+// only when DCHECK is on.
+#if DCHECK_IS_ON()
 TEST_F(HeapTest, PreFinalizerBackingShrinkForbidden) {
   MakeGarbageCollected<PreFinalizerBackingShrinkForbidden>();
   PreciselyCollectGarbage();
@@ -1563,6 +1543,27 @@ TEST(HeapDeathTest, PreFinalizerHashTableBackingExpandForbidden) {
   MakeGarbageCollected<PreFinalizerHashTableBackingExpandForbidden>();
   TestSupportingGC::PreciselyCollectGarbage();
 }
+#endif  // DCHECK_IS_ON()
+
+class PreFinalizerAllocationForbidden
+    : public GarbageCollectedFinalized<PreFinalizerAllocationForbidden> {
+  USING_PRE_FINALIZER(PreFinalizerAllocationForbidden, Dispose);
+
+ public:
+  void Dispose() {
+    EXPECT_FALSE(ThreadState::Current()->IsAllocationAllowed());
+#if DCHECK_IS_ON()
+    EXPECT_DEATH(MakeGarbageCollected<IntWrapper>(1), "");
+#endif  // DCHECK_IS_ON()
+  }
+
+  void Trace(blink::Visitor* visitor) {}
+};
+
+TEST(HeapDeathTest, PreFinalizerAllocationForbidden) {
+  MakeGarbageCollected<PreFinalizerAllocationForbidden>();
+  TestSupportingGC::PreciselyCollectGarbage();
+}
 
 class LargeMixin : public GarbageCollected<LargeMixin>, public Mixin {
   USING_GARBAGE_COLLECTED_MIXIN(LargeMixin);
@@ -4390,32 +4391,6 @@ TEST_F(HeapTest, AllocationDuringFinalization) {
   EXPECT_EQ(32, LargeHeapObject::destructor_calls_);
 }
 
-TEST_F(HeapTest, AllocationDuringPrefinalizer) {
-  ClearOutOldGarbage();
-  IntWrapper::destructor_calls_ = 0;
-  OneKiloByteObject::destructor_calls_ = 0;
-  LargeHeapObject::destructor_calls_ = 0;
-
-  Persistent<IntWrapper> wrapper;
-  MakeGarbageCollected<PreFinalizationAllocator>(&wrapper);
-
-  PreciselyCollectGarbage();
-  EXPECT_EQ(0, IntWrapper::destructor_calls_);
-  EXPECT_EQ(0, OneKiloByteObject::destructor_calls_);
-  EXPECT_EQ(0, LargeHeapObject::destructor_calls_);
-  // Check that the wrapper allocated during finalization is not
-  // swept away and zapped later in the same sweeping phase.
-  EXPECT_EQ(42, wrapper->Value());
-
-  wrapper.Clear();
-  PreciselyCollectGarbage();
-  // The 42 IntWrappers were the ones allocated in the pre-finalizer
-  // of PreFinalizationAllocator and the ones allocated in LargeHeapObject.
-  EXPECT_EQ(42, IntWrapper::destructor_calls_);
-  EXPECT_EQ(512, OneKiloByteObject::destructor_calls_);
-  EXPECT_EQ(32, LargeHeapObject::destructor_calls_);
-}
-
 class SimpleClassWithDestructor {
  public:
   SimpleClassWithDestructor() = default;

third_party/blink/renderer/platform/heap/thread_state.cc

@@ -1241,9 +1241,8 @@ void ThreadState::InvokePreFinalizers() {
   ThreadHeapStatsCollector::Scope stats_scope(
       Heap().stats_collector(), ThreadHeapStatsCollector::kInvokePreFinalizers);
   SweepForbiddenScope sweep_forbidden(this);
-  // Pre finalizers may access unmarked objects but are forbidden from
-  // ressurecting them.
-  ObjectResurrectionForbiddenScope object_resurrection_forbidden(this);
+  // Pre finalizers are forbidden from allocating objects
+  NoAllocationScope no_allocation_scope(this);
 
   // Call the prefinalizers in the opposite order to their registration.
   //