diff --git a/src/coreclr/vm/loaderallocator.cpp b/src/coreclr/vm/loaderallocator.cpp index 58740770b2b644..4e94ddc8680327 100644 --- a/src/coreclr/vm/loaderallocator.cpp +++ b/src/coreclr/vm/loaderallocator.cpp @@ -13,6 +13,8 @@ #endif #include "comcallablewrapper.h" +//#define ENABLE_LOG_LOADER_ALLOCATOR_CLEANUP 1 + #define STUBMANAGER_RANGELIST(stubManager) (stubManager::g_pManager->GetRangeList()) UINT64 LoaderAllocator::cLoaderAllocatorsCreated = 1; @@ -125,6 +127,9 @@ void LoaderAllocator::AddReference() _ASSERTE((m_cReferences > (UINT32)0) && (m_cReferences != (UINT32)-1)); InterlockedIncrement((LONG *)&m_cReferences); +#ifdef ENABLE_LOG_LOADER_ALLOCATOR_CLEANUP + minipal_log_print_info("LoaderAllocator::AddReference LA %d(%p) %d\n", m_nLoaderAllocator, this, (int)m_cReferences.Load()); +#endif } #endif //!DACCESS_COMPILE @@ -162,6 +167,9 @@ BOOL LoaderAllocator::AddReferenceIfAlive() if (cOriginalReferences == cReferencesLocalSnapshot) { // The exchange happened +#ifdef ENABLE_LOG_LOADER_ALLOCATOR_CLEANUP + minipal_log_print_info("LoaderAllocator::AddReferenceIfAlive LA %d(%p) %d\n", this->m_nLoaderAllocator, this, (int)m_cReferences.Load()); +#endif return TRUE; } // Let's spin till we are the only thread to modify this value @@ -191,6 +199,9 @@ BOOL LoaderAllocator::Release() _ASSERTE((m_cReferences > (UINT32)0) && (m_cReferences != (UINT32)-1)); LONG cNewReferences = InterlockedDecrement((LONG *)&m_cReferences); +#ifdef ENABLE_LOG_LOADER_ALLOCATOR_CLEANUP + minipal_log_print_info("LoaderAllocator::Release LA %d(%p) %d\n", this->m_nLoaderAllocator, this, (int)m_cReferences.Load()); +#endif return (cNewReferences == 0); #else //DACCESS_COMPILE @@ -226,6 +237,10 @@ BOOL LoaderAllocator::CheckAddReference_Unlocked(LoaderAllocator *pOtherLA) m_LoaderAllocatorReferences.Add(pOtherLA); // Notify the other LoaderAllocator that a reference exists +#ifdef ENABLE_LOG_LOADER_ALLOCATOR_CLEANUP + minipal_log_print_info("LoaderAllocator::CheckAddReference_Unlocked LA %d(%p) to LA %d(%p)\n", m_nLoaderAllocator, this, pOtherLA->m_nLoaderAllocator, pOtherLA, (int)pOtherLA->m_cReferences.Load()); +#endif + pOtherLA->AddReference(); return TRUE; } @@ -358,45 +373,42 @@ LoaderAllocator * LoaderAllocator::GCLoaderAllocators_RemoveAssemblies(AppDomain // List of LoaderAllocators being deleted LoaderAllocator * pFirstDestroyedLoaderAllocator = NULL; -#if 0 - // Debug logic for debugging the loader allocator gc. + AppDomain::AssemblyIterator i; { + // Iterate through every loader allocator, marking as we go + CrstHolder chLoaderAllocatorReferencesLock(pAppDomain->GetLoaderAllocatorReferencesLock()); + CrstHolder chAssemblyListLock(pAppDomain->GetAssemblyListLock()); + + CollectibleAssemblyHolder pAssembly; + +#ifdef ENABLE_LOG_LOADER_ALLOCATOR_CLEANUP + // Debug logic for debugging the loader allocator gc. /* Iterate through every loader allocator, and print its current state */ - AppDomain::AssemblyIterator iData; - iData = pAppDomain->IterateAssembliesEx((AssemblyIterationFlags)( + i = pAppDomain->IterateAssembliesEx((AssemblyIterationFlags)( kIncludeExecution | kIncludeLoaded | kIncludeCollected)); - CollectibleAssemblyHolder pAssembly; - while (iData.Next_Unlocked(pAssembly.This())) + while (i.Next_Unlocked(pAssembly.This())) { if (pAssembly != NULL) { LoaderAllocator * pLoaderAllocator = pAssembly->GetLoaderAllocator(); if (pLoaderAllocator->IsCollectible()) { - minipal_log_print_info("LA %p ReferencesTo %d\n", pLoaderAllocator, pLoaderAllocator->m_cReferences); + minipal_log_print_info("LA %d(%p) ReferencesTo %d\n", pLoaderAllocator->m_nLoaderAllocator, pLoaderAllocator, (int)pLoaderAllocator->m_cReferences.Load()); LoaderAllocatorSet::Iterator iter = pLoaderAllocator->m_LoaderAllocatorReferences.Begin(); while (iter != pLoaderAllocator->m_LoaderAllocatorReferences.End()) { LoaderAllocator * pAllocator = *iter; - minipal_log_print_info("LARefTo: %p\n", pAllocator); + minipal_log_print_info("LARefTo: %d(%p)\n", pAllocator->m_nLoaderAllocator, pAllocator); iter++; } } } } - } -#endif //0 - - AppDomain::AssemblyIterator i; - { - // Iterate through every loader allocator, marking as we go - CrstHolder chLoaderAllocatorReferencesLock(pAppDomain->GetLoaderAllocatorReferencesLock()); - CrstHolder chAssemblyListLock(pAppDomain->GetAssemblyListLock()); +#endif // ENABLE_LOG_LOADER_ALLOCATOR_CLEANUP i = pAppDomain->IterateAssembliesEx((AssemblyIterationFlags)( kIncludeExecution | kIncludeLoaded | kIncludeCollected)); - CollectibleAssemblyHolder pAssembly; while (i.Next_Unlocked(pAssembly.This())) { @@ -406,7 +418,12 @@ LoaderAllocator * LoaderAllocator::GCLoaderAllocators_RemoveAssemblies(AppDomain if (pLoaderAllocator->IsCollectible()) { if (pLoaderAllocator->IsAlive()) + { + // This mark is a deep mark, it will mark all LoaderAllocators that this one references too (recursively), + // even ones that are not alive. Note that the caller of the current function has decremented reference count + // of the loader allocator we are going to release and also all of its dependencies. pLoaderAllocator->Mark(); + } } } } @@ -508,13 +525,16 @@ void LoaderAllocator::GCLoaderAllocators(LoaderAllocator* pOriginalLoaderAllocat } CONTRACTL_END; +#ifdef ENABLE_LOG_LOADER_ALLOCATOR_CLEANUP + minipal_log_print_info("-------LoaderAllocator::GCLoaderAllocators LA %d(%p) %d\n", pOriginalLoaderAllocator->m_nLoaderAllocator, pOriginalLoaderAllocator, pOriginalLoaderAllocator->m_cReferences.Load()); +#endif // List of LoaderAllocators being deleted LoaderAllocator * pFirstDestroyedLoaderAllocator = NULL; AppDomain* pAppDomain = AppDomain::GetCurrentDomain(); // Collect all LoaderAllocators that don't have anymore DomainAssemblies alive - // Note: that it may not collect our pOriginalLoaderAllocator in case this + // Note: that it will not collect our pOriginalLoaderAllocator in case this // LoaderAllocator hasn't loaded any DomainAssembly. We handle this case in the next loop. // Note: The removed LoaderAllocators are not reachable outside of this function anymore, because we // removed them from the assembly list @@ -550,10 +570,13 @@ void LoaderAllocator::GCLoaderAllocators(LoaderAllocator* pOriginalLoaderAllocat pDomainLoaderAllocatorDestroyIterator = pDomainLoaderAllocatorDestroyIterator->m_pLoaderAllocatorDestroyNext; } - // If the original LoaderAllocator was not processed, it is most likely a LoaderAllocator without any loaded DomainAssembly + // If the original LoaderAllocator was not processed, it is a LoaderAllocator without any loaded DomainAssembly // But we still want to collect it so we add it to the list of LoaderAllocator to destroy - if (!isOriginalLoaderAllocatorFound && !pOriginalLoaderAllocator->IsAlive()) + if (!isOriginalLoaderAllocatorFound && !pOriginalLoaderAllocator->Id()->HasAttachedDynamicAssemblies() && !pOriginalLoaderAllocator->IsAlive()) { +#ifdef ENABLE_LOG_LOADER_ALLOCATOR_CLEANUP + minipal_log_print_info("LoaderAllocator::GCLoaderAllocators FORCED unload due to no DomainAssembly LA %d(%p) %d\n", pOriginalLoaderAllocator->m_nLoaderAllocator, pOriginalLoaderAllocator, (int)pOriginalLoaderAllocator->m_cReferences.Load()); +#endif pOriginalLoaderAllocator->m_pLoaderAllocatorDestroyNext = pFirstDestroyedLoaderAllocator; pFirstDestroyedLoaderAllocator = pOriginalLoaderAllocator; } @@ -562,6 +585,9 @@ void LoaderAllocator::GCLoaderAllocators(LoaderAllocator* pOriginalLoaderAllocat pDomainLoaderAllocatorDestroyIterator = pFirstDestroyedLoaderAllocator; while (pDomainLoaderAllocatorDestroyIterator != NULL) { +#ifdef ENABLE_LOG_LOADER_ALLOCATOR_CLEANUP + minipal_log_print_info("LoaderAllocator::GCLoaderAllocators DESTROY LA %d(%p) %d\n", pDomainLoaderAllocatorDestroyIterator->m_nLoaderAllocator, pDomainLoaderAllocatorDestroyIterator, (int)pDomainLoaderAllocatorDestroyIterator->m_cReferences.Load()); +#endif _ASSERTE(!pDomainLoaderAllocatorDestroyIterator->IsAlive()); DomainAssemblyIterator domainAssemblyIt(pDomainLoaderAllocatorDestroyIterator->m_pFirstDomainAssemblyFromSameALCToDelete); @@ -642,6 +668,9 @@ BOOL LoaderAllocator::Destroy(QCall::LoaderAllocatorHandle pLoaderAllocator) { STRESS_LOG1(LF_CLASSLOADER, LL_INFO100, "Begin LoaderAllocator::Destroy for loader allocator %p\n", reinterpret_cast(static_cast(pLoaderAllocator))); LoaderAllocatorID *pID = pLoaderAllocator->Id(); +#ifdef ENABLE_LOG_LOADER_ALLOCATOR_CLEANUP + minipal_log_print_info("LoaderAllocator::Destroy LA %d(%p) %d\n", pLoaderAllocator->m_nLoaderAllocator, pLoaderAllocator, pLoaderAllocator->m_cReferences.Load()); +#endif { GCX_COOP(); @@ -1044,6 +1073,9 @@ void LoaderAllocator::ActivateManagedTracking() // There is now one external reference to this LoaderAllocator (the managed scout) _ASSERTE(m_cReferences == (UINT32)-1); m_cReferences = (UINT32)1; +#ifdef ENABLE_LOG_LOADER_ALLOCATOR_CLEANUP + minipal_log_print_info("LoaderAllocator::ActivateManagedTracking LA %d(%p) %d\n", m_nLoaderAllocator, this, (int)m_cReferences.Load()); +#endif LOADERALLOCATORREF loaderAllocator = (LOADERALLOCATORREF)ObjectFromHandle(m_hLoaderAllocatorObjectHandle); loaderAllocator->SetNativeLoaderAllocator(this); diff --git a/src/coreclr/vm/loaderallocator.hpp b/src/coreclr/vm/loaderallocator.hpp index 4e8de0ffa6470c..6c876fde167574 100644 --- a/src/coreclr/vm/loaderallocator.hpp +++ b/src/coreclr/vm/loaderallocator.hpp @@ -232,6 +232,14 @@ class LoaderAllocatorID m_pValue = value; }; VOID Init(); + bool HasAttachedDynamicAssemblies() + { + if (m_type == LAT_Assembly && m_pDomainAssembly != NULL) + { + return true; + } + return false; + } LoaderAllocatorType GetType(); VOID AddDomainAssembly(DomainAssembly* pDomainAssembly); DomainAssemblyIterator GetDomainAssemblyIterator(); diff --git a/src/tests/Regressions/coreclr/GitHub_116953/AssemblyA.csproj b/src/tests/Regressions/coreclr/GitHub_116953/AssemblyA.csproj new file mode 100644 index 00000000000000..22aadf1151361a --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_116953/AssemblyA.csproj @@ -0,0 +1,9 @@ + + + BuildOnly + Library + + + + + diff --git a/src/tests/Regressions/coreclr/GitHub_116953/AssemblyB.csproj b/src/tests/Regressions/coreclr/GitHub_116953/AssemblyB.csproj new file mode 100644 index 00000000000000..250a7f76d909f7 --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_116953/AssemblyB.csproj @@ -0,0 +1,13 @@ + + + BuildOnly + Library + + + + + + + + + diff --git a/src/tests/Regressions/coreclr/GitHub_116953/AssemblyC.csproj b/src/tests/Regressions/coreclr/GitHub_116953/AssemblyC.csproj new file mode 100644 index 00000000000000..770493acebeb6d --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_116953/AssemblyC.csproj @@ -0,0 +1,9 @@ + + + BuildOnly + Library + + + + + diff --git a/src/tests/Regressions/coreclr/GitHub_116953/ClassA.cs b/src/tests/Regressions/coreclr/GitHub_116953/ClassA.cs new file mode 100644 index 00000000000000..4fa13c2ee51b93 --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_116953/ClassA.cs @@ -0,0 +1,7 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +namespace AssemblyA; + +public class ClassA +{ +} diff --git a/src/tests/Regressions/coreclr/GitHub_116953/ClassB.cs b/src/tests/Regressions/coreclr/GitHub_116953/ClassB.cs new file mode 100644 index 00000000000000..405910d3e647e1 --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_116953/ClassB.cs @@ -0,0 +1,19 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +using AssemblyC; + +namespace AssemblyB; + +public class ClassB +{ + public string GetMessage() + { + var b = new GenericClass(); + var c = new ClassC(); + return $"Hello from Assembly B! -> {c.GetMessage()} -> {b.ToString()}"; + } +} + +public class GenericClass +{ +} \ No newline at end of file diff --git a/src/tests/Regressions/coreclr/GitHub_116953/ClassC.cs b/src/tests/Regressions/coreclr/GitHub_116953/ClassC.cs new file mode 100644 index 00000000000000..46413ca7dcd3cf --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_116953/ClassC.cs @@ -0,0 +1,11 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +namespace AssemblyC; + +public class ClassC +{ + public string GetMessage() + { + return "Hello from Assembly C!"; + } +} \ No newline at end of file diff --git a/src/tests/Regressions/coreclr/GitHub_116953/test116953.cs b/src/tests/Regressions/coreclr/GitHub_116953/test116953.cs new file mode 100644 index 00000000000000..8ead623d56ddc0 --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_116953/test116953.cs @@ -0,0 +1,126 @@ +// 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.Threading; +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Runtime.Loader; +using Xunit; + +// ALC For AssemblyB +public class AssemblyLoadContextB : AssemblyLoadContext +{ + public AssemblyLoadContextB(string name, bool isCollectible) : base(name, isCollectible) + { + } + + protected override Assembly Load(AssemblyName assemblyName) + { + if (assemblyName.Name == "AssemblyC") + { + Test116953.Log($"AssemblyB is resolving C"); + return Test116953.AssemblyC; + } + + if (assemblyName.Name == "AssemblyA") + { + Test116953.Log($"AssemblyB is resolving A"); + return Test116953.AssemblyA; + } + + return null; + } +} + +public class Test116953 +{ + public static Assembly AssemblyA; + public static Assembly AssemblyC; + + [Fact] + public static void TestEntryPoint() + { + for (int i = 0; i < 3; i++) + { + Log($"Test #{i}"); + RunOneTest(); + } + } + + private static void RunOneTest() + { + // Create three collectible ALCs + var alcB = new AssemblyLoadContextB("ALC_B", isCollectible: true); + var alcC = new AssemblyLoadContext("ALC_C", isCollectible: true); + var alcA = new AssemblyLoadContext("ALC_A", isCollectible: true); + + // Track ALCs with weak references + WeakReference alcCRef = new WeakReference(alcC, trackResurrection: true); + WeakReference alcBRef = new WeakReference(alcB, trackResurrection: true); + WeakReference alcARef = new WeakReference(alcA, trackResurrection: true); + + // Load assembly A + AssemblyA = LoadAssembly(alcA, "AssemblyA"); + + // Load assembly C + AssemblyC = LoadAssembly(alcC, "AssemblyC"); + + // Load assembly B (depends on assemblies A and C) + Assembly assemblyB = LoadAssembly(alcB, "AssemblyB"); + Log($"AssemblyB: {assemblyB}"); + + // Call method in assembly B + Log("\nTesting call to method in assembly B:"); + Type? typeBClass = assemblyB.GetType("AssemblyB.ClassB"); + if (typeBClass != null) + { + object bInstance = Activator.CreateInstance(typeBClass)!; + string resultB = (string)typeBClass.GetMethod("GetMessage").Invoke(bInstance, null); + Log($"B method returns: {resultB}"); + } + else + { + Log("AssemblyB.ClassB not found!"); + } + + AssemblyA = null; + AssemblyC = null; + + // Unload the three ALCs + Log("\nStarting to unload ALCs..."); + + alcA.Unload(); + alcC.Unload(); + alcB.Unload(); + + alcA = null; + alcB = null; + alcC = null; + + for (int i = 0; i < 100 && (alcARef.IsAlive || alcBRef.IsAlive || alcCRef.IsAlive); i++) + { + Thread.Sleep(1); + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + + Log($"ALC_A status: {(alcARef.IsAlive ? "still in memory" : "unloaded")}"); + Log($"ALC_B status: {(alcBRef.IsAlive ? "still in memory" : "unloaded")}"); + Log($"ALC_C status: {(alcCRef.IsAlive ? "still in memory" : "unloaded")}"); + + Log("Test completed"); + } + + private static Assembly LoadAssembly(AssemblyLoadContext alc, string assemblyName) + { + string assemblyDir = Assembly.GetExecutingAssembly().Location; + assemblyDir = Path.GetDirectoryName(assemblyDir)!; + return alc.LoadFromAssemblyPath(Path.Combine(assemblyDir, $"{assemblyName}.dll")); + } + + public static void Log(string message) + { + Console.WriteLine(message); + } +} diff --git a/src/tests/Regressions/coreclr/GitHub_116953/test116953.csproj b/src/tests/Regressions/coreclr/GitHub_116953/test116953.csproj new file mode 100644 index 00000000000000..29d2c10bdc0861 --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_116953/test116953.csproj @@ -0,0 +1,15 @@ + + + 1 + True + + + + + + + + + + +