From c04a1beac1521872312a0e78442ea2805b048916 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:09:53 +0000 Subject: [PATCH 1/4] Fix GenericCache flush sentinel detection and add regression test Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- .../Runtime/CompilerServices/GenericCache.cs | 21 +++- .../GenericVirtualMethod.cs | 95 +++++++++++++++++++ .../GenericVirtualMethod.csproj | 13 +++ .../GenericVirtualMethodUnloaded.cs | 43 +++++++++ .../GenericVirtualMethodUnloaded.csproj | 11 +++ 5 files changed, 178 insertions(+), 5 deletions(-) create mode 100644 src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethod.cs create mode 100644 src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethod.csproj create mode 100644 src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethodUnloaded.cs create mode 100644 src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethodUnloaded.csproj diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/GenericCache.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/GenericCache.cs index 94d9ecdd73bf3f..e807cf52604bc7 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/GenericCache.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/GenericCache.cs @@ -57,6 +57,9 @@ private struct Entry private const uint VERSION_NUM_MASK = (1 << VERSION_NUM_SIZE) - 1; private const int BUCKET_SIZE = 8; + // The number of elements in the sentinel table (see _sentinelTable). + private const int SENTINEL_TABLE_SIZE = 2; + // The fields of this structure are known to coreclr, so if they are updated, you must also update object.h // The actual storage. @@ -74,7 +77,7 @@ private struct Entry // creates a new cache instance public GenericCache(int initialCacheSize, int maxCacheSize) { - Debug.Assert(BitOperations.PopCount((uint)initialCacheSize) == 1 && initialCacheSize > 1); + Debug.Assert(BitOperations.PopCount((uint)initialCacheSize) == 1 && initialCacheSize > SENTINEL_TABLE_SIZE); Debug.Assert(BitOperations.PopCount((uint)maxCacheSize) == 1 && maxCacheSize >= initialCacheSize); _initialCacheSize = initialCacheSize; @@ -83,7 +86,7 @@ public GenericCache(int initialCacheSize, int maxCacheSize) // A trivial 2-elements table used for "flushing" the cache. // Nothing is ever stored in such a small table and identity of the sentinel is not important. // It is required that we are able to allocate this, we may need this in OOM cases. - _sentinelTable = CreateCacheTable(2, throwOnFail: true)!; + _sentinelTable = CreateCacheTable(SENTINEL_TABLE_SIZE, throwOnFail: true)!; _table = #if !DEBUG @@ -138,6 +141,14 @@ private static ref Entry Element(Entry[] table, int index) return ref Unsafe.Add(ref Unsafe.As(ref Unsafe.As(table).Data), index + 1); } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool IsSentinel(Entry[] table) + { + // The sentinel is the only table with SENTINEL_TABLE_SIZE elements. + // NOTE: the actual array is one element longer, since element 0 is used for aux data. + return CacheElementCount(table) == SENTINEL_TABLE_SIZE; + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] internal bool TryGet(TKey key, out TValue? value) { @@ -246,9 +257,9 @@ internal void TrySet(TKey key, TValue value) do { table = _table; - if (table.Length == 2) + if (IsSentinel(table)) { - // 2-element table is used as a sentinel. + // sentinel table is used to indicate that // we did not allocate a real table yet or have flushed it. // try replacing the table, but do not insert anything. MaybeReplaceCacheWithLarger(_lastFlushSize); @@ -322,7 +333,7 @@ internal void TrySet(TKey key, TValue value) // reread tableData after TryGrow. table = _table; - if (table.Length == 2) + if (IsSentinel(table)) { // do not insert into a sentinel. return; diff --git a/src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethod.cs b/src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethod.cs new file mode 100644 index 00000000000000..f8175bc18e80d7 --- /dev/null +++ b/src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethod.cs @@ -0,0 +1,95 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.IO; +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Runtime.Loader; +using Xunit; +using TestLibrary; + +// Repeatedly loads an assembly into a collectible AssemblyLoadContext, invokes a generic +// virtual method on a type from that assembly and unloads the context. +// The results of the generic virtual dispatch are stored in a process-wide cache, which is +// flushed when a collectible context is unloaded. If the flush does not take effect, a stale +// entry could be returned for a new type that happens to reuse the addresses of an unloaded +// one, which results in a hang or a crash. +public class GenericVirtualMethodUnloading +{ + private class TestALC : AssemblyLoadContext + { + public TestALC(int id) : base($"GenericVirtualMethod{id}", isCollectible: true) + { + } + } + + [ActiveIssue("https://github.com/dotnet/runtimelab/issues/155: Collectible assemblies", typeof(Utilities), nameof(Utilities.IsNativeAot))] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34072", TestRuntimes.Mono)] + [Fact] + public static void CallGenericVirtualMethodAcrossUnloads() + { + string payloadPath = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "GenericVirtualMethodUnloaded.dll"); + + for (int iteration = 0; iteration < 10; iteration++) + { + WeakReference context = InvokeAndUnload(payloadPath, iteration); + for (int collection = 0; context.IsAlive && collection < 10; collection++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + + Assert.False(context.IsAlive, $"Context {iteration} did not unload."); + VerifyVirtualDispatchCacheIsEmpty(); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static WeakReference InvokeAndUnload(string payloadPath, int iteration) + { + TestALC alc = new TestALC(iteration); + WeakReference weakAlc = new WeakReference(alc, trackResurrection: false); + + Assembly payload = alc.LoadFromAssemblyPath(payloadPath); + Type machineType = payload.GetType("DerivedMachine", throwOnError: true); + Type stateType = payload.GetType("MarkerState", throwOnError: true); + Type baseType = payload.GetType("Machine", throwOnError: true); + + object machine = Activator.CreateInstance(machineType); + MethodInfo change = baseType.GetMethod("Change").MakeGenericMethod(stateType); + + TargetInvocationException ex = Assert.Throws(() => change.Invoke(machine, null)); + Assert.Equal("ExpectedException", ex.InnerException.GetType().FullName); + + alc.Unload(); + return weakAlc; + } + + // Unloading a collectible context flushes the virtual function pointer cache, so that + // targets belonging to the unloaded context cannot be returned by a later lookup. + // Check that the cache is indeed empty after the unload. + private static void VerifyVirtualDispatchCacheIsEmpty() + { + Type helpersType = typeof(object).Assembly.GetType("System.Runtime.CompilerServices.VirtualDispatchHelpers"); + if (helpersType == null) + { + // The cache is specific to CoreCLR. + return; + } + + object cache = helpersType.GetField("s_virtualFunctionPointerCache", BindingFlags.NonPublic | BindingFlags.Static).GetValue(null); + Array table = (Array)cache.GetType().GetField("_table", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(cache); + + FieldInfo infoField = table.GetType().GetElementType().GetField("_info", BindingFlags.NonPublic | BindingFlags.Instance); + FieldInfo versionField = infoField.FieldType.GetField("_version", BindingFlags.NonPublic | BindingFlags.Instance); + + // element 0 of the table holds the auxiliary data of the table, the entries start at index 1. + // A nonzero version means the entry is in use. + for (int i = 1; i < table.Length; i++) + { + object entryInfo = infoField.GetValue(table.GetValue(i)); + Assert.Equal(0u, (uint)versionField.GetValue(entryInfo)); + } + } +} diff --git a/src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethod.csproj b/src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethod.csproj new file mode 100644 index 00000000000000..e3b09983593d9e --- /dev/null +++ b/src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethod.csproj @@ -0,0 +1,13 @@ + + + + true + + + + + + + + + diff --git a/src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethodUnloaded.cs b/src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethodUnloaded.cs new file mode 100644 index 00000000000000..e6d557339e4f87 --- /dev/null +++ b/src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethodUnloaded.cs @@ -0,0 +1,43 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; + +public class State; +public sealed class MarkerState : State; +public sealed class ExpectedException : Exception; + +public abstract class Machine +{ + private readonly Dictionary _states = new(); + + public void Change() where T : State + { + _ = GetOrCreate(); + } + + private T GetOrCreate() where T : State + { + if (!_states.TryGetValue(typeof(T), out State state)) + { + state = Construct(); + _states[typeof(T)] = state; + } + + return (T)state; + } + + protected virtual T Construct() where T : State + { + return Activator.CreateInstance(); + } +} + +public sealed class DerivedMachine : Machine +{ + protected override T Construct() + { + throw new ExpectedException(); + } +} diff --git a/src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethodUnloaded.csproj b/src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethodUnloaded.csproj new file mode 100644 index 00000000000000..2f23b7016fded0 --- /dev/null +++ b/src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethodUnloaded.csproj @@ -0,0 +1,11 @@ + + + Library + + + + + + + + From 4e6307d676a9173e00979f41614a77c50ab421b4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 28 Aug 2026 00:50:01 +0000 Subject: [PATCH 2/4] Compare table length with SENTINEL_TABLE_SIZE + 1 in IsSentinel Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- .../src/System/Runtime/CompilerServices/GenericCache.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/GenericCache.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/GenericCache.cs index e807cf52604bc7..9e35b2af93ac9f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/GenericCache.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/GenericCache.cs @@ -146,7 +146,7 @@ private static bool IsSentinel(Entry[] table) { // The sentinel is the only table with SENTINEL_TABLE_SIZE elements. // NOTE: the actual array is one element longer, since element 0 is used for aux data. - return CacheElementCount(table) == SENTINEL_TABLE_SIZE; + return table.Length == SENTINEL_TABLE_SIZE + 1; } [MethodImpl(MethodImplOptions.AggressiveInlining)] From bcd91950b137587a1a862aa7c0283e40d987973f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 28 Aug 2026 01:00:24 +0000 Subject: [PATCH 3/4] Use SENTINEL_TABLE_SIZE/IsSentinel in CastCache for consistency Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com> --- .../Runtime/CompilerServices/CastCache.cs | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastCache.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastCache.cs index 880fd3d3850661..fc241bcd7f33a7 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastCache.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastCache.cs @@ -22,6 +22,9 @@ internal unsafe struct CastCache private const uint VERSION_NUM_MASK = (1 << VERSION_NUM_SIZE) - 1; private const int BUCKET_SIZE = 8; + // The number of elements in the sentinel table (see s_sentinelTable). + private const int SENTINEL_TABLE_SIZE = 2; + // nothing is ever stored into this, so we can use a static instance. private static int[]? s_sentinelTable; @@ -36,7 +39,7 @@ internal unsafe struct CastCache public CastCache(int initialCacheSize, int maxCacheSize) { - Debug.Assert(BitOperations.PopCount((uint)initialCacheSize) == 1 && initialCacheSize > 1); + Debug.Assert(BitOperations.PopCount((uint)initialCacheSize) == 1 && initialCacheSize > SENTINEL_TABLE_SIZE); Debug.Assert(BitOperations.PopCount((uint)maxCacheSize) == 1 && maxCacheSize >= initialCacheSize); _initialCacheSize = initialCacheSize; @@ -45,7 +48,7 @@ public CastCache(int initialCacheSize, int maxCacheSize) // A trivial 2-elements table used for "flushing" the cache. // Nothing is ever stored in such a small table and identity of the sentinel is not important. // It is required that we are able to allocate this, we may need this in OOM cases. - s_sentinelTable ??= CreateCastCache(2, throwOnFail: true); + s_sentinelTable ??= CreateCastCache(SENTINEL_TABLE_SIZE, throwOnFail: true); _table = #if !DEBUG @@ -123,6 +126,13 @@ private static ref int TableMask(ref int tableData) return ref Unsafe.Add(ref tableData, 1); } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool IsSentinel(ref int tableData) + { + // The sentinel is the only table with SENTINEL_TABLE_SIZE elements. + return TableMask(ref tableData) == SENTINEL_TABLE_SIZE - 1; + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] private static ref uint VictimCounter(ref int tableData) { @@ -256,9 +266,9 @@ internal void TrySet(nuint source, nuint target, bool result) do { tableData = ref TableData(_table); - if (TableMask(ref tableData) == 1) + if (IsSentinel(ref tableData)) { - // 2-element table is used as a sentinel. + // the sentinel table is used to indicate that // we did not allocate a real table yet or have flushed it. // try replacing the table, but do not insert anything. MaybeReplaceCacheWithLarger(_lastFlushSize); @@ -331,7 +341,7 @@ internal void TrySet(nuint source, nuint target, bool result) // reread tableData after TryGrow. tableData = ref TableData(_table); - if (TableMask(ref tableData) == 1) + if (IsSentinel(ref tableData)) { // do not insert into a sentinel. return; From 6cc1605dd53c37c3f8c95585cb9eda22a0f8a475 Mon Sep 17 00:00:00 2001 From: Vladimir Sadov Date: Fri, 28 Aug 2026 08:53:25 -0700 Subject: [PATCH 4/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../GenericVirtualMethod/GenericVirtualMethod.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethod.cs b/src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethod.cs index f8175bc18e80d7..eb221fc350497d 100644 --- a/src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethod.cs +++ b/src/tests/Loader/CollectibleAssemblies/GenericVirtualMethod/GenericVirtualMethod.cs @@ -56,11 +56,12 @@ private static WeakReference InvokeAndUnload(string payloadPath, int iteration) Type stateType = payload.GetType("MarkerState", throwOnError: true); Type baseType = payload.GetType("Machine", throwOnError: true); - object machine = Activator.CreateInstance(machineType); - MethodInfo change = baseType.GetMethod("Change").MakeGenericMethod(stateType); + object machine = Activator.CreateInstance(machineType)!; + MethodInfo change = baseType.GetMethod("Change")!.MakeGenericMethod(stateType); TargetInvocationException ex = Assert.Throws(() => change.Invoke(machine, null)); - Assert.Equal("ExpectedException", ex.InnerException.GetType().FullName); + Assert.NotNull(ex.InnerException); + Assert.Equal("ExpectedException", ex.InnerException!.GetType().FullName); alc.Unload(); return weakAlc;