fix: Reserve promotion budget before allocation, refund on failure

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp

@@ -1027,6 +1027,13 @@ inline bool ShenandoahHeap::should_retry_allocation(size_t original_full_gc_coun
 }
 
 HeapWord* ShenandoahHeap::allocate_memory_work(ShenandoahAllocRequest& req, bool& in_new_region) {
+  // Reserve the promotion budget up front so it is enforced atomically without the heap lock.
+  // If the reserve is exhausted, deny the promotion rather than overshoot it; the reservation
+  // is refunded below if the allocation itself fails.
+  if (req.is_promotion() && !old_generation()->try_expend_promoted(req.size() << LogHeapWordSize)) {
+    return nullptr;
+  }
+
   HeapWord* result = _allocator->allocate(req, in_new_region);
 
   if (result != nullptr) {
@@ -1038,17 +1045,13 @@ HeapWord* ShenandoahHeap::allocate_memory_work(ShenandoahAllocRequest& req, bool
       if (req.is_lab_alloc()) {
         old_generation()->configure_plab_for_current_thread(req);
       } else if (req.is_promotion()) {
-        const size_t actual_size = req.actual_size() * HeapWordSize;
-        // The object has already been promoted into old-gen, so account for it unconditionally.
-        // can_allocate() gated this promotion against the reserve under the heap lock, but the
-        // lock is released before we get here; a conditional reservation could fail under
-        // concurrent expends and silently lose accounting, leading to unbounded over-promotion.
-        log_debug(gc, plab)("Expend shared promotion of %zu bytes", actual_size);
-        old_generation()->expend_promoted(actual_size);
+        log_debug(gc, plab)("Expend shared promotion of %zu bytes", req.actual_size() * HeapWordSize);
       }
     }
+  } else if (req.is_promotion()) {
+    // Allocation failed, so refund the promotion budget reserved above.
+    old_generation()->unexpend_promoted(req.size() << LogHeapWordSize);
   }
-
   return result;
 }
 

src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.cpp

@@ -206,10 +206,6 @@ bool ShenandoahOldGeneration::try_expend_promoted(size_t increment) {
   return false;
 }
 
-size_t ShenandoahOldGeneration::expend_promoted(size_t increment) {
-  return _promoted_expended.add_then_fetch(increment);
-}
-
 size_t ShenandoahOldGeneration::unexpend_promoted(size_t decrement) {
   return _promoted_expended.sub_then_fetch(decrement);
 }

src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.hpp

@@ -116,12 +116,6 @@ public:
   // Use this to gate a promotion decision before promoting.
   bool try_expend_promoted(size_t increment);
 
-  // Unconditionally account `increment` bytes that have already been promoted into old-gen.
-  // Unlike try_expend_promoted, this never fails: the promotion has happened, so the budget
-  // must reflect it to stay truthful (it may briefly overshoot the reserve under concurrency).
-  // Lock-free: safe to call without the heap lock.
-  size_t expend_promoted(size_t increment);
-
   // This is used to return unused memory from a retired promotion LAB
   size_t unexpend_promoted(size_t decrement);
 

src/hotspot/share/gc/shenandoah/shenandoahPartitionAllocator.cpp

@@ -44,7 +44,7 @@ HeapWord* ShenandoahPartitionAllocator<PARTITION>::allocate(ShenandoahAllocReque
 
   // OldCollector: verify old generation has room before attempting allocation.
   if constexpr (PARTITION == ShenandoahFreeSetPartitionId::OldCollector) {
-    if (!ShenandoahHeap::heap()->old_generation()->can_allocate(req)) {
+    if (!req.is_promotion() && !ShenandoahHeap::heap()->old_generation()->can_allocate(req)) {
       return nullptr;
     }
   }

test/hotspot/gtest/gc/shenandoah/test_shenandoahOldGeneration.cpp

@@ -179,16 +179,4 @@ TEST_VM_F(ShenandoahOldGenerationTest, test_try_expend_promoted_should_increase_
   EXPECT_EQ(expended_before + 128, expended_after) << "Should expend promotion";
 }
 
-TEST_VM_F(ShenandoahOldGenerationTest, test_expend_promoted_accounts_unconditionally) {
-  SKIP_IF_NOT_SHENANDOAH();
-  // try_expend_promoted gates a promotion decision and must fail past the reserve...
-  const size_t over = old->get_promoted_reserve() + 1;
-  size_t expended_before = old->get_promoted_expended();
-  EXPECT_FALSE(old->try_expend_promoted(over)) << "Reservation must fail past the reserve";
-  EXPECT_EQ(expended_before, old->get_promoted_expended()) << "Failed reservation must not change accounting";
-  // ...but expend_promoted accounts bytes already promoted unconditionally, even past the reserve.
-  EXPECT_EQ(expended_before + over, old->expend_promoted(over))
-      << "Already-promoted bytes must be accounted even past the reserve";
-}
-
 #undef SKIP_IF_NOT_SHENANDOAH