[release/8.0] Fix crash when calling AssemblyLoadContext.Unload twice (…

…#91346) * 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. * Add regression test --------- Co-authored-by: Jan Vorlicek <janvorli@microsoft.com>
dotnet · Aug 30, 2023 · 7174092 · 7174092
1 parent 8ec6101
commit 7174092
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 12 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
@@ -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<long, WeakReference<AssemblyLoadContext>> allContexts = AllContexts;
-            lock (allContexts)
+            if (previousState == InternalState.Alive)
             {
-                allContexts.Remove(_id);
+                Dictionary<long, WeakReference<AssemblyLoadContext>> allContexts = AllContexts;
+                lock (allContexts)
+                {
+                    allContexts.Remove(_id);
+                }
             }
         }
 

diff --git 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<InvalidOperationException>(() => 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()
         {