From 86d6a2e05eb52ea2c603a06bce838a56d5ae507b Mon Sep 17 00:00:00 2001
From: Axel Boldt-Christmas <aboldtch@openjdk.org>
Date: Fri, 29 Aug 2025 07:35:03 +0000
Subject: [PATCH] 8366147: ZGC:
 ZPageAllocator::cleanup_failed_commit_single_partition may leak memory

Reviewed-by: stefank, sjohanss, jsikstro
---
 src/hotspot/share/gc/z/zPageAllocator.cpp     | 23 +++++++++++--------
 .../hotspot/jtreg/gc/z/TestCommitFailure.java |  8 +++++++
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/src/hotspot/share/gc/z/zPageAllocator.cpp b/src/hotspot/share/gc/z/zPageAllocator.cpp
index d8f653cc7d0..9f484394521 100644
--- a/src/hotspot/share/gc/z/zPageAllocator.cpp
+++ b/src/hotspot/share/gc/z/zPageAllocator.cpp
@@ -1092,7 +1092,8 @@ void ZPartition::free_memory_alloc_failed(ZMemoryAllocation* allocation) {
     freed += vmem.size();
     _cache.insert(vmem);
   }
-  assert(allocation->harvested() + allocation->committed_capacity() == freed, "must have freed all");
+  assert(allocation->harvested() + allocation->committed_capacity() == freed, "must have freed all"
+         " %zu + %zu == %zu", allocation->harvested(), allocation->committed_capacity(), freed);
 
   // Adjust capacity to reflect the failed capacity increase
   const size_t remaining = allocation->size() - freed;
@@ -1909,23 +1910,27 @@ void ZPageAllocator::cleanup_failed_commit_single_partition(ZSinglePartitionAllo
   ZMemoryAllocation* const allocation = single_partition_allocation->allocation();
 
   assert(allocation->commit_failed(), "Must have failed to commit");
+  assert(allocation->partial_vmems()->is_empty(), "Invariant for single partition commit failure");
 
-  const size_t committed = allocation->committed_capacity();
-  const ZVirtualMemory non_harvested_vmem = vmem.last_part(allocation->harvested());
-  const ZVirtualMemory committed_vmem = non_harvested_vmem.first_part(committed);
-  const ZVirtualMemory non_committed_vmem = non_harvested_vmem.last_part(committed);
+  // For a single partition we have unmapped the harvested memory before we
+  // started committing, and moved its physical memory association to the start
+  // of the vmem. As such, the partial_vmems is empty. All the harvested and
+  // partially successfully committed memory is mapped in the first part of vmem.
+  const size_t harvested_and_committed_capacity = allocation->harvested() + allocation->committed_capacity();
+  const ZVirtualMemory succeeded_vmem = vmem.first_part(harvested_and_committed_capacity);
+  const ZVirtualMemory failed_vmem = vmem.last_part(harvested_and_committed_capacity);
 
-  if (committed_vmem.size() > 0) {
+  if (succeeded_vmem.size() > 0) {
     // Register the committed and mapped memory. We insert the committed
     // memory into partial_vmems so that it will be inserted into the cache
     // in a subsequent step.
-    allocation->partial_vmems()->append(committed_vmem);
+    allocation->partial_vmems()->append(succeeded_vmem);
   }
 
   // Free the virtual and physical memory we fetched to use but failed to commit
   ZPartition& partition = allocation->partition();
-  partition.free_physical(non_committed_vmem);
-  partition.free_virtual(non_committed_vmem);
+  partition.free_physical(failed_vmem);
+  partition.free_virtual(failed_vmem);
 }
 
 void ZPageAllocator::cleanup_failed_commit_multi_partition(ZMultiPartitionAllocation* multi_partition_allocation, const ZVirtualMemory& vmem) {
diff --git a/test/hotspot/jtreg/gc/z/TestCommitFailure.java b/test/hotspot/jtreg/gc/z/TestCommitFailure.java
index c1babe7eb83..3cee84d4f95 100644
--- a/test/hotspot/jtreg/gc/z/TestCommitFailure.java
+++ b/test/hotspot/jtreg/gc/z/TestCommitFailure.java
@@ -23,6 +23,14 @@
 
 package gc.z;
 
+/*
+ * @test id=Normal
+ * @requires vm.gc.Z & vm.debug
+ * @summary Test ZGC graceful failure when a commit fails
+ * @library / /test/lib
+ * @run driver gc.z.TestCommitFailure
+ */
+
 /*
  * @test id=ZFakeNUMA
  * @requires vm.gc.Z & vm.debug