From e972be7793e64baaba8e8c8642a60e334290a28d Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Wed, 3 Jun 2026 09:12:14 -0700
Subject: [PATCH] fix: Account shared promotions unconditionally to avoid lost
 budget

---
 src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp   | 10 ++++++----
 .../share/gc/shenandoah/shenandoahOldGeneration.cpp  |  4 ++++
 .../share/gc/shenandoah/shenandoahOldGeneration.hpp  |  7 +++++++
 .../gc/shenandoah/test_shenandoahOldGeneration.cpp   | 12 ++++++++++++
 4 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
index 0d62c91b406..74514044bab 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
@@ -1039,10 +1039,12 @@ HeapWord* ShenandoahHeap::allocate_memory_work(ShenandoahAllocRequest& req, bool
         old_generation()->configure_plab_for_current_thread(req);
       } else if (req.is_promotion()) {
         const size_t actual_size = req.actual_size() * HeapWordSize;
-        // Atomic reserve so concurrent GC workers can't over-expend the promotion budget without the lock.
-        if (!old_generation()->try_expend_promoted(actual_size)) {
-          log_debug(gc, plab)("Shared promotion of %zu bytes exceeded promotion reserve", actual_size);
-        }
+        // 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);
       }
     }
   }
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.cpp b/src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.cpp
index 52121e4b2ed..33768f7dded 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.cpp
@@ -206,6 +206,10 @@ 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);
 }
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.hpp b/src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.hpp
index eedf14ce62f..2b66a12dfb5 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.hpp
@@ -113,8 +113,15 @@ public:
 
   // Atomically reserve `increment` bytes of promotion budget. Returns true iff the full amount
   // was reserved without exceeding the reserve. Lock-free: safe to call without the heap lock.
+  // 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);
 
diff --git a/test/hotspot/gtest/gc/shenandoah/test_shenandoahOldGeneration.cpp b/test/hotspot/gtest/gc/shenandoah/test_shenandoahOldGeneration.cpp
index 483464fba17..853b669de5e 100644
--- a/test/hotspot/gtest/gc/shenandoah/test_shenandoahOldGeneration.cpp
+++ b/test/hotspot/gtest/gc/shenandoah/test_shenandoahOldGeneration.cpp
@@ -179,4 +179,16 @@ 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