diff --git a/src/coreclr/gc/interface.cpp b/src/coreclr/gc/interface.cpp index 00d5f77f1a5e3f..2d1f31b52dbc79 100644 --- a/src/coreclr/gc/interface.cpp +++ b/src/coreclr/gc/interface.cpp @@ -2073,7 +2073,6 @@ size_t GCHeap::ApproxTotalBytesInUse(BOOL small_heap_only) generation* gen = pGenGCHeap->generation_of (0); size_t gen0_frag = generation_free_list_space (gen) + generation_free_obj_space (gen); uint8_t* current_alloc_allocated = pGenGCHeap->alloc_allocated; - heap_segment* current_eph_seg = pGenGCHeap->ephemeral_heap_segment; size_t gen0_size = 0; #ifdef USE_REGIONS heap_segment* gen0_seg = generation_start_segment (gen); @@ -2083,19 +2082,17 @@ size_t GCHeap::ApproxTotalBytesInUse(BOOL small_heap_only) current_alloc_allocated : heap_segment_allocated (gen0_seg); gen0_size += end - heap_segment_mem (gen0_seg); - if (gen0_seg == current_eph_seg) - { - break; - } - gen0_seg = heap_segment_next (gen0_seg); } #else //USE_REGIONS // For segments ephemeral seg does not change. + heap_segment* current_eph_seg = pGenGCHeap->ephemeral_heap_segment; gen0_size = current_alloc_allocated - heap_segment_mem (current_eph_seg); #endif //USE_REGIONS - totsize = gen0_size - gen0_frag; + // Defense-in-depth clamp: gen0 frag counters are updated by the allocator under a different lock. + // This read can observe a transiently inconsistent snapshot; avoid underflow. + totsize = (gen0_size > gen0_frag) ? (gen0_size - gen0_frag) : 0; int stop_gen_index = max_generation; diff --git a/src/tests/GC/API/GC/GetTotalMemoryConcurrent.cs b/src/tests/GC/API/GC/GetTotalMemoryConcurrent.cs new file mode 100644 index 00000000000000..933269e842967c --- /dev/null +++ b/src/tests/GC/API/GC/GetTotalMemoryConcurrent.cs @@ -0,0 +1,135 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// GC.GetTotalMemory(false) must never return a negative value. +// +// The value bottoms out in ApproxTotalBytesInUse, which for gen0 computes gen0_size - gen0_frag +// as an unsigned quantity. gen0 fragmentation (free-list + free-object space) is a per-generation +// total that spans every gen0 region, so gen0_size must span every gen0 region as well. When gen0 +// retains a region past the ephemeral one (for example a region held in place by a pinned object), +// its span has to be included; otherwise fragmentation exceeds the counted span, the subtraction +// underflows, and the managed API surfaces the wrapped value as a negative long. +// +// This test drives concurrent allocation with a large, continuously-refreshed set of pinned +// objects (to force such retained gen0 regions) while repeatedly probing GetTotalMemory on another +// thread. Without the accounting fix it returns a negative value in well under a second; with the +// fix the value stays non-negative. + +using System; +using System.Diagnostics; +using System.Runtime.InteropServices; +using System.Threading; +using Xunit; + +public class GetTotalMemoryConcurrent +{ + private static volatile bool s_stop; + + [Fact] + public static void TestEntryPoint() + { + // A couple of seconds is far more than enough: the unfixed runtime fails almost immediately + // (typically within the first ~100K probes, well under a second). Even a single allocating + // thread is sufficient to build the region layout that triggers the miscount, so this does + // not depend on the number of processors. + const int DurationSeconds = 2; + int workers = Math.Max(2, Math.Min(8, Environment.ProcessorCount)); + + // Reset in case this ever runs more than once in the same process: the worker threads exit + // as soon as s_stop is set, so a stale 'true' would leave the heap idle and mask the bug. + s_stop = false; + + var threads = new Thread[workers]; + for (int i = 0; i < workers; i++) + { + threads[i] = new Thread(PinChurn) { IsBackground = true }; + threads[i].Start(); + } + + long probes = 0; + long minObserved = long.MaxValue; + long negative = 0; + bool allStopped = true; + var sw = Stopwatch.StartNew(); + try + { + while (sw.Elapsed.TotalSeconds < DurationSeconds) + { + long total = GC.GetTotalMemory(false); + probes++; + if (total < minObserved) + { + minObserved = total; + } + + if (total < 0) + { + negative = total; + break; + } + } + } + finally + { + s_stop = true; + foreach (Thread t in threads) + { + if (!t.Join(TimeSpan.FromSeconds(5))) + { + allStopped = false; + } + } + } + + Console.WriteLine($"probes={probes}, min observed={minObserved}, negative={negative}"); + Assert.True(negative >= 0, $"GC.GetTotalMemory(false) returned a negative value: {negative}"); + Assert.True(allStopped, "A worker thread did not stop within the join timeout."); + } + + // Keeps a large ring of pinned tiny objects alive, refreshing the oldest one each iteration, and + // floods gen0 with throwaway garbage in between. The pins prevent their gen0 regions from being + // compacted, so those regions are swept and retained with large free lists while the live set + // stays small - exactly the shape that makes gen0 fragmentation exceed the counted gen0 span. + private static void PinChurn() + { + var rng = new Random(Environment.CurrentManagedThreadId); + const int RingSize = 4096; + var ring = new GCHandle[RingSize]; + int slot = 0; + long sink = 0; + + try + { + while (!s_stop) + { + if (ring[slot].IsAllocated) + { + ring[slot].Free(); + } + ring[slot] = GCHandle.Alloc(new byte[24], GCHandleType.Pinned); + slot = (slot + 1) % RingSize; + + for (int j = 0; j < 96; j++) + { + byte[] junk = new byte[rng.Next(8, 256)]; + sink += junk.Length; + } + } + } + finally + { + for (int i = 0; i < RingSize; i++) + { + if (ring[i].IsAllocated) + { + ring[i].Free(); + } + } + } + + if (sink == long.MaxValue) + { + Console.WriteLine(sink); + } + } +} diff --git a/src/tests/GC/API/GC/GetTotalMemoryConcurrent.csproj b/src/tests/GC/API/GC/GetTotalMemoryConcurrent.csproj new file mode 100644 index 00000000000000..f0115d9f2dd851 --- /dev/null +++ b/src/tests/GC/API/GC/GetTotalMemoryConcurrent.csproj @@ -0,0 +1,14 @@ + + + + true + 1 + + + PdbOnly + true + + + + +