diff --git a/src/coreclr/vm/clsload.cpp b/src/coreclr/vm/clsload.cpp index 7862eef313478f..cde177bf2ff266 100644 --- a/src/coreclr/vm/clsload.cpp +++ b/src/coreclr/vm/clsload.cpp @@ -78,14 +78,14 @@ NameHandle::NameHandle(Module* pModule, mdToken token) : // This method determines the "loader module" for an instantiated type // or method. The rule must ensure that any types involved in the // instantiated type or method do not outlive the loader module itself -// with respect to app-domain unloading (e.g. MyList can't be +// with respect to module unloading (e.g. MyList can't be // put in the module of MyList if MyList's assembly is -// app-domain-neutral but MyType's assembly is app-domain-specific). +// non-collectible but MyType's assembly is collectible). // The rule we use is: // // * Pick the first type in the class instantiation, followed by -// method instantiation, whose loader module is non-shared (app-domain-bound) -// * If no type is app-domain-bound, return the module containing the generic type itself +// method instantiation, whose loader allocator is collectible and has the highest creation number. +// * If no type is in collectible assembly, return the module containing the generic type itself. // // Some useful effects of this rule (for ngen purposes) are: // @@ -113,18 +113,15 @@ PTR_Module ClassLoader::ComputeLoaderModuleWorker( } CONTRACT_END + // No generic instantiation, return the definition module if (classInst.IsEmpty() && methodInst.IsEmpty()) RETURN PTR_Module(pDefinitionModule); - Module *pLoaderModule = NULL; - - if (pDefinitionModule) - { - if (pDefinitionModule->IsCollectible()) - goto ComputeCollectibleLoaderModule; - pLoaderModule = pDefinitionModule; - } + // Use the definition module as the loader module by default + Module *pLoaderModule = pDefinitionModule; + // If any of generic type arguments are in collectible module, + // we use a generic procedure. for (DWORD i = 0; i < classInst.GetNumArgs(); i++) { TypeHandle classArg = classInst[i]; @@ -135,6 +132,8 @@ PTR_Module ClassLoader::ComputeLoaderModuleWorker( pLoaderModule = pModule; } + // If any of generic method arguments are in collectible module, + // we also use a generic procedure. for (DWORD i = 0; i < methodInst.GetNumArgs(); i++) { TypeHandle methodArg = methodInst[i]; @@ -155,14 +154,18 @@ PTR_Module ClassLoader::ComputeLoaderModuleWorker( if (FALSE) { ComputeCollectibleLoaderModule: - LoaderAllocator *pLoaderAllocatorOfDefiningType = NULL; - LoaderAllocator *pOldestLoaderAllocator = NULL; - Module *pOldestLoaderModule = NULL; - UINT64 oldestFoundAge = 0; + Module *pLatestLoaderModule = NULL; + UINT64 latestFoundNumber = 0; DWORD classArgsCount = classInst.GetNumArgs(); DWORD totalArgsCount = classArgsCount + methodInst.GetNumArgs(); - if (pDefinitionModule != NULL) pLoaderAllocatorOfDefiningType = pDefinitionModule->GetLoaderAllocator(); + // If loader allocator of the defining type is collectible, we use it + // and its creation number as the starting age. + if (pDefinitionModule != NULL && pDefinitionModule->IsCollectible()) + { + pLatestLoaderModule = pDefinitionModule; + latestFoundNumber = pDefinitionModule->GetLoaderAllocator()->GetCreationNumber(); + } for (DWORD i = 0; i < totalArgsCount; i++) { @@ -176,22 +179,18 @@ PTR_Module ClassLoader::ComputeLoaderModuleWorker( Module *pModuleCheck = arg.GetLoaderModule(); LoaderAllocator *pLoaderAllocatorCheck = pModuleCheck->GetLoaderAllocator(); - if (pLoaderAllocatorCheck != pLoaderAllocatorOfDefiningType && - pLoaderAllocatorCheck->IsCollectible() && - pLoaderAllocatorCheck->GetCreationNumber() > oldestFoundAge) + if (pLoaderAllocatorCheck->IsCollectible() && + pLoaderAllocatorCheck->GetCreationNumber() > latestFoundNumber) { - pOldestLoaderModule = pModuleCheck; - pOldestLoaderAllocator = pLoaderAllocatorCheck; - oldestFoundAge = pLoaderAllocatorCheck->GetCreationNumber(); + pLatestLoaderModule = pModuleCheck; + latestFoundNumber = pLoaderAllocatorCheck->GetCreationNumber(); } } - // Only if we didn't find a different loader allocator than the defining loader allocator do we - // use the defining loader allocator - if (pOldestLoaderModule != NULL) - pLoaderModule = pOldestLoaderModule; - else - pLoaderModule = pDefinitionModule; + // Use the module of the latest found collectible loader allocator. + // If nothing was found, then by default we use the defining module. + if (pLatestLoaderModule != NULL) + pLoaderModule = pLatestLoaderModule; } RETURN PTR_Module(pLoaderModule); } diff --git a/src/libraries/System.Runtime.Loader/tests/AssemblyLoadContextTest.cs b/src/libraries/System.Runtime.Loader/tests/AssemblyLoadContextTest.cs index f5b2785d823eea..00e924fcb47b0a 100644 --- a/src/libraries/System.Runtime.Loader/tests/AssemblyLoadContextTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/AssemblyLoadContextTest.cs @@ -1,14 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using Xunit; -using System; -using System.Collections.Generic; using System.IO; -using System.Linq; using System.Reflection; using System.Reflection.Emit; -using System.Threading.Tasks; +using Xunit; namespace System.Runtime.Loader.Tests { @@ -189,7 +185,7 @@ public static void DefaultAssemblyLoadContext_Properties() Assert.Contains("\"Default\"", alc.ToString()); Assert.Contains("System.Runtime.Loader.DefaultAssemblyLoadContext", alc.ToString()); Assert.Contains(alc, AssemblyLoadContext.All); - Assert.Contains(Assembly.GetCallingAssembly(), alc.Assemblies); + Assert.Contains(typeof(int).Assembly, alc.Assemblies); } [Fact] diff --git a/src/libraries/System.Runtime.Loader/tests/CollectibleAssemblyLoadContextTest.cs b/src/libraries/System.Runtime.Loader/tests/CollectibleAssemblyLoadContextTest.cs index f357ba72fd7280..9a717078bc1741 100644 --- a/src/libraries/System.Runtime.Loader/tests/CollectibleAssemblyLoadContextTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/CollectibleAssemblyLoadContextTest.cs @@ -32,7 +32,7 @@ static void CreateAndLoadContext(CollectibleChecker checker, bool unloadTwice = } [Fact] - [ActiveIssue("https://github.com/mono/mono/issues/15142", TestRuntimes.Mono)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34072", TestRuntimes.Mono)] public static void Unload_CollectibleWithNoAssemblyLoaded() { // Use a collectible ALC + Unload @@ -44,7 +44,7 @@ public static void Unload_CollectibleWithNoAssemblyLoaded() } [Fact] - [ActiveIssue("https://github.com/mono/mono/issues/15142", TestRuntimes.Mono)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34072", TestRuntimes.Mono)] public static void DoubleUnload_CollectibleWithNoAssemblyLoaded() { // Use a collectible ALC + Unload @@ -91,7 +91,7 @@ public TestBase(int numContexts) } [MethodImpl(MethodImplOptions.NoInlining)] - public void CreateContextAndLoadAssembly(int contextIndex = 0) + public void CreateContextAndLoadAssembly(int contextIndex = 0, string testClassName = "TestClass") { var asmName = new AssemblyName(TestAssembly); _contexts[contextIndex] = new ResourceAssemblyLoadContext(true) { LoadBy = LoadBy.Path }; @@ -99,7 +99,7 @@ public void CreateContextAndLoadAssembly(int contextIndex = 0) Assembly asm = _contexts[contextIndex].LoadFromAssemblyName(asmName); Assert.NotNull(asm); - _testClassTypes[contextIndex] = asm.DefinedTypes.FirstOrDefault(t => t.Name == "TestClass"); + _testClassTypes[contextIndex] = asm.DefinedTypes.FirstOrDefault(t => t.Name == testClassName); Assert.NotNull(_testClassTypes[contextIndex]); _checker.SetAssemblyLoadContext(contextIndex, _contexts[contextIndex]); @@ -126,7 +126,7 @@ class CollectibleWithOneAssemblyLoadedTest : TestBase } [Fact] - [ActiveIssue("https://github.com/mono/mono/issues/15142", TestRuntimes.Mono)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34072", TestRuntimes.Mono)] public static void Unload_CollectibleWithOneAssemblyLoaded() { // Use a collectible ALC + Load an assembly by path + Unload @@ -153,7 +153,7 @@ public void Execute() } [Fact] - [ActiveIssue("https://github.com/mono/mono/issues/15142", TestRuntimes.Mono)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34072", TestRuntimes.Mono)] public static void Unload_CollectibleWithOneAssemblyLoadedWithStatic() { // Use a collectible ALC + Load an assembly by path + New Instance + Static reference + Unload @@ -185,7 +185,7 @@ public void CheckInstanceUnloaded() } [Fact] - [ActiveIssue("https://github.com/mono/mono/issues/15142", TestRuntimes.Mono)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34072", TestRuntimes.Mono)] public static void Unload_CollectibleWithOneAssemblyLoadedWithWeakReferenceToType() { // Use a collectible ALC + Load an assembly by path + WeakReference on the Type + Unload @@ -219,7 +219,7 @@ public void CheckInstanceUnloaded() } [Fact] - [ActiveIssue("https://github.com/mono/mono/issues/15142", TestRuntimes.Mono)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34072", TestRuntimes.Mono)] public static void Unload_CollectibleWithOneAssemblyLoadedWithWeakReferenceToInstance() { // Use a collectible ALC + Load an assembly by path + WeakReference on an instance of a Type + Unload @@ -271,7 +271,7 @@ public void CheckTypeUnloaded() } [Fact] - [ActiveIssue("https://github.com/mono/mono/issues/15142", TestRuntimes.Mono)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34072", TestRuntimes.Mono)] public static void Unload_CollectibleWithOneAssemblyLoadedWithStrongReferenceToType() { // Use a collectible ALC + Load an assembly by path + Strong reference on the Type + Unload @@ -332,7 +332,7 @@ public void CheckInstanceUnloaded() } [Fact] - [ActiveIssue("https://github.com/mono/mono/issues/15142", TestRuntimes.Mono)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34072", TestRuntimes.Mono)] public static void Unload_CollectibleWithOneAssemblyLoadedWithStrongReferenceToInstance() { // Use a collectible ALC + Load an assembly by path + Strong reference on an instance of a Type + Unload @@ -366,7 +366,7 @@ public void Execute() } [Fact] - [ActiveIssue("https://github.com/mono/mono/issues/15142", TestRuntimes.Mono)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34072", TestRuntimes.Mono)] public static void Unload_CollectibleWithTwoAssemblies() { // Use a collectible ALC + Load two assemblies (path + stream) + Unload @@ -427,7 +427,7 @@ public void CheckContextUnloaded2() } [Fact] - [ActiveIssue("https://github.com/mono/mono/issues/15142", TestRuntimes.Mono)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34072", TestRuntimes.Mono)] public static void Unload_TwoCollectibleWithOneAssemblyAndOneInstanceReferencingAnother() { // We create 2 collectible ALC, load one assembly in each, create one instance in each, reference one instance from ALC1 to ALC2 @@ -453,6 +453,70 @@ public static void Unload_TwoCollectibleWithOneAssemblyAndOneInstanceReferencing test.CheckContextUnloaded1(); } + class TwoCollectibleWithOneAssemblyAndOneInstanceReferencingAnotherThroughGenericStaticTest : TestBase + { + public TwoCollectibleWithOneAssemblyAndOneInstanceReferencingAnotherThroughGenericStaticTest() : base(2) + { + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public void Execute() + { + // Make an instance in ALC2 and assign it to a static field in a generic type declared in ALC1, + // but with a genric parameter in ALC2 and thus instantiated in ALC2. + Type type = _testClassTypes[0].MakeGenericType(new [] {_testClassTypes[1]}); + FieldInfo field = type.GetField("StaticObjectRef"); + Assert.NotNull(field); + + object instance = Activator.CreateInstance(_testClassTypes[1]); + field.SetValue(null, instance); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public void CheckNotUnloaded() + { + // None of the AssemblyLoadContexts should be unloaded + _checker.GcAndCheck(0); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public void CheckContextUnloaded1() + { + // The AssemblyLoadContext should now be unloaded + _checker.GcAndCheck(); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public void CheckContextUnloaded2() + { + // The AssemblyLoadContext should now be unloaded + _checker.GcAndCheck(1); + } + } + + // Test may fail when running on a different runtime + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34072", TestRuntimes.Mono)] + public static void Unload_TwoCollectibleWithOneAssemblyAndOneInstanceReferencingAnotherThroughGenericStatic() + { + // We create 2 collectible ALC, load one assembly in each, create one instance in the ALC2, + // reference it from ALC1 to ALC2 using a generic static variable. + // unload ALC2 -> check that instance is not there and we receive one unload + // unload ALC1 -> we should receive 1 unload + + var test = new TwoCollectibleWithOneAssemblyAndOneInstanceReferencingAnotherThroughGenericStaticTest(); + test.CreateContextAndLoadAssembly(0, "GenericTestClass`1"); + test.CreateContextAndLoadAssembly(1); + + test.Execute(); + + test.UnloadAndClearContext(1); + test.CheckContextUnloaded2(); + + test.UnloadAndClearContext(0); + test.CheckContextUnloaded1(); + } + private class CollectibleChecker { private readonly int _expectedCount; diff --git a/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Test.Assembly/TestClass.cs b/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Test.Assembly/TestClass.cs index 822dde52d9b464..4a54cb27b7c33b 100644 --- a/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Test.Assembly/TestClass.cs +++ b/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Test.Assembly/TestClass.cs @@ -36,4 +36,11 @@ public class COMClass { } } + + // Used to test LoaderAllocator used for a generic type instance allocation + public class GenericTestClass + { + // Allow to store a static reference (either an instance of the ALC or an instance outside of it) + public static T StaticObjectRef; + } }