cppgc: Introduce DisallowGarbageCollectionScope

Allows for prohibiting GCs and will result in a crash in case a GC
finalization event is triggered.

Complements NoGarbageCollectionScope which ignores GC finalization
events.

Bug: chromium:1056170
Change-Id: Ie2a72a8675462b24692225af17c8f284318337ba
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2656260Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72413}

include/cppgc/heap-consistency.h

@@ -147,14 +147,59 @@ class V8_EXPORT HeapState final {
    */
   static bool IsMarking(HeapHandle& heap_handle);
 
+ private:
+  HeapState() = delete;
+};
+
+/**
+ * Disallows garbage collection finalizations. Any garbage collection triggers
+ * result in a crash when in this scope.
+ *
+ * Note that the garbage collector already covers paths that can lead to garbage
+ * collections, so user code does not require checking
+ * `IsGarbageCollectionAllowed()` before allocations.
+ */
+class V8_EXPORT V8_NODISCARD DisallowGarbageCollectionScope final {
+  CPPGC_STACK_ALLOCATED();
+
+ public:
+  /**
+   * \returns whether garbage collections are currently allowed.
+   */
+  static bool IsGarbageCollectionAllowed(HeapHandle& heap_handle);
+
+  /**
+   * Enters a disallow garbage collection scope. Must be paired with `Leave()`.
+   * Prefer a scope instance of `DisallowGarbageCollectionScope`.
+   *
+   * \param heap_handle The corresponding heap.
+   */
+  static void Enter(HeapHandle& heap_handle);
+
   /**
+   * LEaves a disallow garbage collection scope. Must be paired with `Enter()`.
+   * Prefer a scope instance of `DisallowGarbageCollectionScope`.
+   *
    * \param heap_handle The corresponding heap.
-   * \returns true if allocations are allowed, and false otherwise.
    */
-  static bool IsAllocationAllowed(HeapHandle& heap_handle);
+  static void Leave(HeapHandle& heap_handle);
+
+  /**
+   * Constructs a scoped object that automatically enters and leaves a disallow
+   * garbage collection scope based on its lifetime.
+   *
+   * \param heap_handle The corresponding heap.
+   */
+  explicit DisallowGarbageCollectionScope(HeapHandle& heap_handle);
+  ~DisallowGarbageCollectionScope();
+
+  DisallowGarbageCollectionScope(const DisallowGarbageCollectionScope&) =
+      delete;
+  DisallowGarbageCollectionScope& operator=(
+      const DisallowGarbageCollectionScope&) = delete;
 
  private:
-  HeapState() = delete;
+  HeapHandle& heap_handle_;
 };
 
 /**

src/heap/cppgc-js/cpp-heap.cc

@@ -258,6 +258,7 @@ bool CppHeap::AdvanceTracing(double deadline_in_ms) {
 bool CppHeap::IsTracingDone() { return marking_done_; }
 
 void CppHeap::EnterFinalPause(EmbedderStackState stack_state) {
+  CHECK(!in_disallow_gc_scope());
   cppgc::internal::StatsCollector::EnabledScope stats_scope(
       AsBase(), cppgc::internal::StatsCollector::kAtomicMark);
   is_in_final_pause_ = true;
@@ -274,14 +275,12 @@ void CppHeap::TraceEpilogue(TraceSummary* trace_summary) {
   {
     cppgc::internal::StatsCollector::EnabledScope stats_scope(
         AsBase(), cppgc::internal::StatsCollector::kAtomicMark);
-    cppgc::internal::ObjectAllocator::NoAllocationScope no_allocation_scope_(
-        object_allocator_);
+    cppgc::subtle::DisallowGarbageCollectionScope disallow_gc_scope(*this);
     marker_->LeaveAtomicPause();
     is_in_final_pause_ = false;
   }
   {
-    cppgc::internal::ObjectAllocator::NoAllocationScope no_allocation_scope_(
-        object_allocator_);
+    cppgc::subtle::DisallowGarbageCollectionScope disallow_gc_scope(*this);
     prefinalizer_handler()->InvokePreFinalizers();
   }
   marker_.reset();

src/heap/cppgc/heap-base.cc

@@ -92,6 +92,7 @@ void HeapBase::AdvanceIncrementalGarbageCollectionOnAllocationIfNeeded() {
 void HeapBase::Terminate() {
   DCHECK(!IsMarking());
   DCHECK(!in_no_gc_scope());
+  CHECK(!in_disallow_gc_scope());
 
   sweeper().FinishIfRunning();
 
@@ -116,6 +117,7 @@ void HeapBase::Terminate() {
   } while (strong_persistent_region_.NodesInUse() > 0);
 
   object_allocator().Terminate();
+  disallow_gc_scope_++;
 }
 
 }  // namespace internal

src/heap/cppgc/heap-base.h

@@ -32,6 +32,7 @@ class Stack;
 
 namespace cppgc {
 namespace subtle {
+class DisallowGarbageCollectionScope;
 class NoGarbageCollectionScope;
 }  // namespace subtle
 
@@ -148,6 +149,8 @@ class V8_EXPORT_PRIVATE HeapBase : public cppgc::HeapHandle {
   // destructors. Exceeding the loop bound results in a crash.
   void Terminate();
 
+  bool in_disallow_gc_scope() const { return disallow_gc_scope_ > 0; }
+
  protected:
   virtual void FinalizeIncrementalGarbageCollectionIfNeeded(
       cppgc::Heap::StackState) = 0;
@@ -182,11 +185,13 @@ class V8_EXPORT_PRIVATE HeapBase : public cppgc::HeapHandle {
 #endif
 
   size_t no_gc_scope_ = 0;
+  size_t disallow_gc_scope_ = 0;
 
   const StackSupport stack_support_;
 
   friend class MarkerBase::IncrementalMarkingTask;
   friend class testing::TestWithHeap;
+  friend class cppgc::subtle::DisallowGarbageCollectionScope;
   friend class cppgc::subtle::NoGarbageCollectionScope;
 };
 

src/heap/cppgc/heap-consistency.cc

@@ -11,6 +11,36 @@
 namespace cppgc {
 namespace subtle {
 
+// static
+bool DisallowGarbageCollectionScope::IsGarbageCollectionAllowed(
+    cppgc::HeapHandle& heap_handle) {
+  auto& heap_base = internal::HeapBase::From(heap_handle);
+  return !heap_base.in_disallow_gc_scope();
+}
+
+// static
+void DisallowGarbageCollectionScope::Enter(cppgc::HeapHandle& heap_handle) {
+  auto& heap_base = internal::HeapBase::From(heap_handle);
+  heap_base.disallow_gc_scope_++;
+}
+
+// static
+void DisallowGarbageCollectionScope::Leave(cppgc::HeapHandle& heap_handle) {
+  auto& heap_base = internal::HeapBase::From(heap_handle);
+  DCHECK_GT(heap_base.disallow_gc_scope_, 0);
+  heap_base.disallow_gc_scope_--;
+}
+
+DisallowGarbageCollectionScope::DisallowGarbageCollectionScope(
+    cppgc::HeapHandle& heap_handle)
+    : heap_handle_(heap_handle) {
+  Enter(heap_handle);
+}
+
+DisallowGarbageCollectionScope::~DisallowGarbageCollectionScope() {
+  Leave(heap_handle_);
+}
+
 // static
 void NoGarbageCollectionScope::Enter(cppgc::HeapHandle& heap_handle) {
   auto& heap_base = internal::HeapBase::From(heap_handle);
@@ -38,11 +68,5 @@ bool HeapState::IsMarking(HeapHandle& heap_handle) {
   return heap_base.marker();
 }
 
-// static
-bool HeapState::IsAllocationAllowed(HeapHandle& heap_handle) {
-  const auto& heap_base = internal::HeapBase::From(heap_handle);
-  return heap_base.object_allocator().is_allocation_allowed();
-}
-
 }  // namespace subtle
 }  // namespace cppgc

src/heap/cppgc/heap.cc

@@ -171,16 +171,17 @@ void Heap::StartGarbageCollection(Config config) {
 void Heap::FinalizeGarbageCollection(Config::StackState stack_state) {
   DCHECK(IsMarking());
   DCHECK(!in_no_gc_scope());
+  CHECK(!in_disallow_gc_scope());
   config_.stack_state = stack_state;
   {
     // This guards atomic pause marking, meaning that no internal method or
     // external callbacks are allowed to allocate new objects.
-    ObjectAllocator::NoAllocationScope no_allocation_scope_(object_allocator_);
+    cppgc::subtle::DisallowGarbageCollectionScope no_gc_scope(*this);
     marker_->FinishMarking(stack_state);
   }
   {
     // Pre finalizers are forbidden from allocating objects.
-    ObjectAllocator::NoAllocationScope no_allocation_scope_(object_allocator_);
+    cppgc::subtle::DisallowGarbageCollectionScope no_gc_scope(*this);
     prefinalizer_handler_->InvokePreFinalizers();
   }
   marker_.reset();

src/heap/cppgc/marker.cc

@@ -6,6 +6,7 @@
 
 #include <memory>
 
+#include "include/cppgc/heap-consistency.h"
 #include "include/cppgc/internal/process-heap.h"
 #include "include/cppgc/platform.h"
 #include "src/heap/cppgc/heap-object-header.h"
@@ -267,8 +268,7 @@ void MarkerBase::LeaveAtomicPause() {
   is_marking_started_ = false;
   {
     // Weakness callbacks are forbidden from allocating objects.
-    ObjectAllocator::NoAllocationScope no_allocation_scope_(
-        heap_.object_allocator());
+    cppgc::subtle::DisallowGarbageCollectionScope disallow_gc_scope(heap_);
     ProcessWeakness();
   }
   g_process_mutex.Pointer()->Unlock();

src/heap/cppgc/object-allocator.cc

@@ -119,7 +119,7 @@ void* ObjectAllocator::OutOfLineAllocateImpl(NormalPageSpace* space,
   DCHECK_EQ(0, size & kAllocationMask);
   DCHECK_LE(kFreeListEntrySize, size);
   // Out-of-line allocation allows for checking this is all situations.
-  CHECK(is_allocation_allowed());
+  CHECK(!in_disallow_gc_scope());
 
   // 1. If this allocation is big enough, allocate a large object.
   if (size >= kLargeObjectSizeThreshold) {
@@ -194,18 +194,10 @@ void ObjectAllocator::ResetLinearAllocationBuffers() {
 
 void ObjectAllocator::Terminate() {
   ResetLinearAllocationBuffers();
-  // OutOfLineAllocateImpl checks is_allocation_allowed() unconditionally.
-  no_allocation_scope_++;
 }
 
-ObjectAllocator::NoAllocationScope::NoAllocationScope(
-    ObjectAllocator& allocator)
-    : allocator_(allocator) {
-  allocator.no_allocation_scope_++;
-}
-
-ObjectAllocator::NoAllocationScope::~NoAllocationScope() {
-  allocator_.no_allocation_scope_--;
+bool ObjectAllocator::in_disallow_gc_scope() const {
+  return raw_heap_->heap()->in_disallow_gc_scope();
 }
 
 }  // namespace internal

src/heap/cppgc/object-allocator.h

@@ -31,22 +31,6 @@ class PageBackend;
 
 class V8_EXPORT_PRIVATE ObjectAllocator final : public cppgc::AllocationHandle {
  public:
-  // NoAllocationScope is used in debug mode to catch unwanted allocations. E.g.
-  // allocations during GC.
-  class V8_EXPORT_PRIVATE V8_NODISCARD NoAllocationScope final {
-    CPPGC_STACK_ALLOCATED();
-
-   public:
-    explicit NoAllocationScope(ObjectAllocator&);
-    ~NoAllocationScope();
-
-    NoAllocationScope(const NoAllocationScope&) = delete;
-    NoAllocationScope& operator=(const NoAllocationScope&) = delete;
-
-   private:
-    ObjectAllocator& allocator_;
-  };
-
   ObjectAllocator(RawHeap* heap, PageBackend* page_backend,
                   StatsCollector* stats_collector);
 
@@ -59,9 +43,9 @@ class V8_EXPORT_PRIVATE ObjectAllocator final : public cppgc::AllocationHandle {
   // Terminate the allocator. Subsequent allocation calls result in a crash.
   void Terminate();
 
-  bool is_allocation_allowed() const { return no_allocation_scope_ == 0; }
-
  private:
+  bool in_disallow_gc_scope() const;
+
   // Returns the initially tried SpaceType to allocate an object of |size| bytes
   // on. Returns the largest regular object size bucket for large objects.
   inline static RawHeap::RegularSpaceType GetInitialSpaceIndexForSize(
@@ -76,11 +60,10 @@ class V8_EXPORT_PRIVATE ObjectAllocator final : public cppgc::AllocationHandle {
   RawHeap* raw_heap_;
   PageBackend* page_backend_;
   StatsCollector* stats_collector_;
-  size_t no_allocation_scope_ = 0;
 };
 
 void* ObjectAllocator::AllocateObject(size_t size, GCInfoIndex gcinfo) {
-  DCHECK(is_allocation_allowed());
+  DCHECK(!in_disallow_gc_scope());
   const size_t allocation_size =
       RoundUp<kAllocationGranularity>(size + sizeof(HeapObjectHeader));
   const RawHeap::RegularSpaceType type =
@@ -91,7 +74,7 @@ void* ObjectAllocator::AllocateObject(size_t size, GCInfoIndex gcinfo) {
 
 void* ObjectAllocator::AllocateObject(size_t size, GCInfoIndex gcinfo,
                                       CustomSpaceIndex space_index) {
-  DCHECK(is_allocation_allowed());
+  DCHECK(!in_disallow_gc_scope());
   const size_t allocation_size =
       RoundUp<kAllocationGranularity>(size + sizeof(HeapObjectHeader));
   return AllocateObjectOnSpace(

src/heap/cppgc/write-barrier.cc

@@ -4,6 +4,7 @@
 
 #include "include/cppgc/internal/write-barrier.h"
 
+#include "include/cppgc/heap-consistency.h"
 #include "include/cppgc/internal/pointer-policies.h"
 #include "src/heap/cppgc/globals.h"
 #include "src/heap/cppgc/heap-object-header.h"
@@ -81,8 +82,7 @@ void WriteBarrier::DijkstraMarkingBarrierRangeSlow(
     return;
   }
 
-  ObjectAllocator::NoAllocationScope no_allocation(
-      heap_base.object_allocator());
+  cppgc::subtle::DisallowGarbageCollectionScope disallow_gc_scope(heap_base);
   const char* array = static_cast<const char*>(first_element);
   while (number_of_elements-- > 0) {
     trace_callback(&heap_base.marker()->Visitor(), array);

test/unittests/heap/cppgc/heap-unittest.cc

@@ -174,14 +174,15 @@ TEST_F(GCHeapTest, NoGarbageCollectionScope) {
   EXPECT_EQ(epoch_after_gc, epoch_before);
 }
 
-TEST_F(GCHeapTest, IsAllocationAllowed) {
+TEST_F(GCHeapTest, IsGarbageCollectionAllowed) {
   EXPECT_TRUE(
-      subtle::HeapState::IsAllocationAllowed(GetHeap()->GetHeapHandle()));
+      subtle::DisallowGarbageCollectionScope::IsGarbageCollectionAllowed(
+          GetHeap()->GetHeapHandle()));
   {
-    ObjectAllocator::NoAllocationScope no_allocation(
-        Heap::From(GetHeap())->object_allocator());
+    subtle::DisallowGarbageCollectionScope disallow_gc(*Heap::From(GetHeap()));
     EXPECT_FALSE(
-        subtle::HeapState::IsAllocationAllowed(GetHeap()->GetHeapHandle()));
+        subtle::DisallowGarbageCollectionScope::IsGarbageCollectionAllowed(
+            GetHeap()->GetHeapHandle()));
   }
 }