From 4b45849b762a16752f8ef83ea04007c75e5aa636 Mon Sep 17 00:00:00 2001 From: William Kemper Date: Thu, 30 Apr 2026 18:03:51 +0000 Subject: [PATCH] 8381154: Shenandoah: Region states could be read in wrong order for comparisons Reviewed-by: shade, ruili --- src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp | 10 ++++++---- .../share/gc/shenandoah/shenandoahHeapRegion.hpp | 4 +++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp b/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp index 1807383123b..fdce385e0f6 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp @@ -223,7 +223,9 @@ ShenandoahFreeSetPartitionId ShenandoahFreeSet::prepare_to_promote_in_place(size } inline bool ShenandoahFreeSet::can_allocate_from(ShenandoahHeapRegion *r) const { - return r->is_empty() || (r->is_trash() && !_heap->is_concurrent_weak_root_in_progress()); + const auto state = r->state(); + return ShenandoahHeapRegion::is_empty_state(state) + || (ShenandoahHeapRegion::is_trash(state) && !_heap->is_concurrent_weak_root_in_progress()); } inline bool ShenandoahFreeSet::can_allocate_from(size_t idx) const { @@ -666,7 +668,7 @@ void ShenandoahRegionPartitions::retire_range_from_partition( #ifdef ASSERT ShenandoahHeapRegion* r = ShenandoahHeap::heap()->get_region(idx); assert (in_free_set(partition, idx), "Must be in partition to remove from partition"); - assert(r->is_empty() || r->is_trash(), "Region must be empty or trash"); + assert(r->is_empty_or_trash(), "Region must be empty or trash"); #endif _membership[int(partition)].clear_bit(idx); } @@ -2822,7 +2824,7 @@ size_t ShenandoahFreeSet::reserve_regions(size_t to_reserve, size_t to_reserve_o // be collected in the near future. if (r->is_trash() || !r->is_affiliated()) { // OLD regions that have available memory are already in the old_collector free set. - assert(r->is_empty() || r->is_trash(), "Not affiliated implies region %zu is empty", r->index()); + assert(r->is_empty_or_trash(), "Not affiliated implies region %zu is empty", r->index()); if (idx < old_collector_low_idx) { old_collector_low_idx = idx; } @@ -3166,7 +3168,7 @@ void ShenandoahFreeSet::log_status() { size_t free = alloc_capacity(r); max = MAX2(max, free); size_t used_in_region = r->used(); - if (r->is_empty() || r->is_trash()) { + if (r->is_empty_or_trash()) { used_in_region = 0; total_free_ext += free; if (last_idx + 1 == idx) { diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp b/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp index 1054a23ea28..6b5cf3b6874 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp @@ -197,11 +197,13 @@ public: bool is_regular() const { return state() == _regular; } bool is_humongous_continuation() const { return state() == _humongous_cont; } bool is_regular_pinned() const { return state() == _pinned; } - bool is_trash() const { return state() == _trash; } + bool is_trash() const { return is_trash(state()); } // Derived state predicates (boolean combinations of individual states) + bool static is_trash(RegionState state) { return state == _trash; } bool static is_empty_state(RegionState state) { return state == _empty_committed || state == _empty_uncommitted; } bool static is_humongous_start_state(RegionState state) { return state == _humongous_start || state == _pinned_humongous_start; } + bool is_empty_or_trash() const { auto cur_state = state(); return is_empty_state(cur_state) || cur_state == _trash; } bool is_empty() const { return is_empty_state(this->state()); } bool is_active() const { auto cur_state = state(); return !is_empty_state(cur_state) && cur_state != _trash; } bool is_humongous_start() const { return is_humongous_start_state(state()); }