From c0b25cd704fc5eb90b39bc10e2724ddd1392fac0 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Tue, 29 Aug 2023 19:31:41 +0200 Subject: [PATCH 1/2] Fix crash when calling AssemblyLoadContext.Unload twice The AssemblyLoadContext.InitiateUnload method was not handling multiple calls correctly. Instead of making the extra calls dummy, it was calling the native runtime helper again. This change fixes it by simply ensuring that the runtime helper is called just once. --- .../Runtime/Loader/AssemblyLoadContext.cs | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.cs index b8b4ba086ad69..59123e42fb52a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.cs @@ -135,25 +135,32 @@ private void InitiateUnload() { RaiseUnloadEvent(); + InternalState previousState; + // When in Unloading state, we are not supposed to be called on the finalizer // as the native side is holding a strong reference after calling Unload lock (_unloadLock) { - Debug.Assert(_state == InternalState.Alive); - - var thisStrongHandle = GCHandle.Alloc(this, GCHandleType.Normal); - var thisStrongHandlePtr = GCHandle.ToIntPtr(thisStrongHandle); - // The underlying code will transform the original weak handle - // created by InitializeLoadContext to a strong handle - PrepareForAssemblyLoadContextRelease(_nativeAssemblyLoadContext, thisStrongHandlePtr); + previousState = _state; + if (previousState == InternalState.Alive) + { + var thisStrongHandle = GCHandle.Alloc(this, GCHandleType.Normal); + var thisStrongHandlePtr = GCHandle.ToIntPtr(thisStrongHandle); + // The underlying code will transform the original weak handle + // created by InitializeLoadContext to a strong handle + PrepareForAssemblyLoadContextRelease(_nativeAssemblyLoadContext, thisStrongHandlePtr); - _state = InternalState.Unloading; + _state = InternalState.Unloading; + } } - Dictionary> allContexts = AllContexts; - lock (allContexts) + if (previousState == InternalState.Alive) { - allContexts.Remove(_id); + Dictionary> allContexts = AllContexts; + lock (allContexts) + { + allContexts.Remove(_id); + } } } From 531f09e0db6340016fda994ddb455248902cbfff Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Wed, 30 Aug 2023 17:25:13 +0200 Subject: [PATCH 2/2] Add regression test --- .../CollectibleAssemblyLoadContextTest.cs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.Loader/tests/CollectibleAssemblyLoadContextTest.cs b/src/libraries/System.Runtime.Loader/tests/CollectibleAssemblyLoadContextTest.cs index 90527c3d0be9d..83d1c9aebf61f 100644 --- a/src/libraries/System.Runtime.Loader/tests/CollectibleAssemblyLoadContextTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/CollectibleAssemblyLoadContextTest.cs @@ -16,12 +16,16 @@ public partial class AssemblyLoadContextTest // Tests related to Collectible assemblies [MethodImpl(MethodImplOptions.NoInlining)] - static void CreateAndLoadContext(CollectibleChecker checker) + static void CreateAndLoadContext(CollectibleChecker checker, bool unloadTwice = false) { var alc = new ResourceAssemblyLoadContext(true); checker.SetAssemblyLoadContext(0, alc); alc.Unload(); + if (unloadTwice) + { + alc.Unload(); + } // Check that any attempt to load an assembly after an explicit Unload will fail Assert.Throws(() => alc.LoadFromAssemblyPath(Path.GetFullPath("none.dll"))); @@ -39,6 +43,19 @@ public static void Unload_CollectibleWithNoAssemblyLoaded() checker.GcAndCheck(); } + [Fact] + [ActiveIssue("https://github.com/mono/mono/issues/15142", TestRuntimes.Mono)] + public static void DoubleUnload_CollectibleWithNoAssemblyLoaded() + { + // Use a collectible ALC + Unload + // Check that we receive the Unloading event + + var checker = new CollectibleChecker(1); + CreateAndLoadContext(checker, unloadTwice: true); + checker.GcAndCheck(); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsPreciseGcSupported))] public static void Finalizer_CollectibleWithNoAssemblyLoaded() {