Disable noncollectible alc finalization (dotnet/coreclr#21189)

* Disable noncollectible AssemblyLoadContext finalization

The constructor of AssemblyLoadContext was missing a call to
GC.SuppressFinalize for non-collectible AssemblyLoadContext. That means
that the finalizer can be executed and unloading being initialized even
for non-collectible AssemblyLoadContext.
This change fixes that.

* SuppressFinalize non-collectible AssemblyLoadContext

The finalizer for the non-collectible AssemblyLoadContext is never
called and so it should not be in the finalization queue.

* Handle finalizer call without constructor executed

Check if the constructor was executed before we run the finalizer code.

Commit migrated from https://github.com/dotnet/coreclr/commit/ac732ffb6d2ff4d2bec72ac5ca0785ace4ace20a

diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.cs
index 8158b3f..f6f27b0 100644 (file)
--- a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.cs
+++ b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.cs
@@ -24,13 +24,13 @@ namespace System.Runtime.Loader
         private static bool _isProcessExiting;
 
         // Id used by contextsToUnload
-        private readonly long id;
+        private readonly long _id;
 
         // synchronization primitive to protect against usage of this instance while unloading
-        private readonly object unloadLock = new object();
+        private readonly object _unloadLock;
 
         // Indicates the state of this ALC (Alive or in Unloading state)
-        private InternalState state;
+        private InternalState _state;
 
         [DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)]
         private static extern IntPtr InitializeAssemblyLoadContext(IntPtr ptrAssemblyLoadContext, bool fRepresentsTPALoadContext, bool isCollectible);
@@ -65,6 +65,16 @@ namespace System.Runtime.Loader
         {
             // Initialize the VM side of AssemblyLoadContext if not already done.
             IsCollectible = isCollectible;
+            // The _unloadLock needs to be assigned after the IsCollectible to ensure proper behavior of the finalizer
+            // even in case the following allocation fails or the thread is aborted between these two lines.
+            _unloadLock = new object();
+
+            if (!isCollectible)
+            {
+                // For non collectible AssemblyLoadContext, the finalizer should never be called and thus the AssemblyLoadContext should not
+                // be on the finalizer queue.
+                GC.SuppressFinalize(this);
+            }
 
             // Add this instance to the list of alive ALC
             lock (ContextsToUnload)
@@ -83,17 +93,23 @@ namespace System.Runtime.Loader
                 Resolving = null;
                 Unloading = null;
 
-                id = _nextId++;
-                ContextsToUnload.Add(id, new WeakReference<AssemblyLoadContext>(this, true));
+                _id = _nextId++;
+                ContextsToUnload.Add(_id, new WeakReference<AssemblyLoadContext>(this, true));
             }
         }
 
         ~AssemblyLoadContext()
         {
-            // Only valid for a Collectible ALC. Non-collectible ALCs have the finalizer suppressed.
-            // We get here only in case the explicit Unload was not initiated.
-            Debug.Assert(state != InternalState.Unloading);
-            InitiateUnload();
+            // Use the _unloadLock as a guard to detect the corner case when the constructor of the AssemblyLoadContext was not executed
+            // e.g. due to the JIT failing to JIT it.
+            if (_unloadLock != null)
+            {
+                // Only valid for a Collectible ALC. Non-collectible ALCs have the finalizer suppressed.
+                Debug.Assert(IsCollectible);
+                // We get here only in case the explicit Unload was not initiated.
+                Debug.Assert(_state != InternalState.Unloading);
+                InitiateUnload();
+            }
         }
 
         private void InitiateUnload()
@@ -104,11 +120,11 @@ namespace System.Runtime.Loader
 
             // 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)
+            lock (_unloadLock)
             {
                 if (!_isProcessExiting)
                 {
-                    Debug.Assert(state == InternalState.Alive);
+                    Debug.Assert(_state == InternalState.Alive);
 
                     var thisStrongHandle = GCHandle.Alloc(this, GCHandleType.Normal);
                     var thisStrongHandlePtr = GCHandle.ToIntPtr(thisStrongHandle);
@@ -117,14 +133,14 @@ namespace System.Runtime.Loader
                     PrepareForAssemblyLoadContextRelease(m_pNativeAssemblyLoadContext, thisStrongHandlePtr);
                 }
 
-                state = InternalState.Unloading;
+                _state = InternalState.Unloading;
             }
 
             if (!_isProcessExiting)
             {
                 lock (ContextsToUnload)
                 {
-                    ContextsToUnload.Remove(id);
+                    ContextsToUnload.Remove(_id);
                 }
             }
         }
@@ -146,7 +162,7 @@ namespace System.Runtime.Loader
                 throw new ArgumentNullException(nameof(assemblyPath));
             }
 
-            lock (unloadLock)
+            lock (_unloadLock)
             {
                 VerifyIsAlive();
                 if (PathInternal.IsPartiallyQualified(assemblyPath))
@@ -168,7 +184,7 @@ namespace System.Runtime.Loader
                 throw new ArgumentNullException(nameof(nativeImagePath));
             }
 
-            lock (unloadLock)
+            lock (_unloadLock)
             {
                 VerifyIsAlive();
 
@@ -207,7 +223,7 @@ namespace System.Runtime.Loader
             {
                 throw new BadImageFormatException(SR.BadImageFormat_BadILFormat);
             }
-            lock (unloadLock)
+            lock (_unloadLock)
             {
                 VerifyIsAlive();
 
@@ -256,7 +272,7 @@ namespace System.Runtime.Loader
 
         private void VerifyIsAlive()
         {
-            if (state != InternalState.Alive)
+            if (_state != InternalState.Alive)
             {
                 throw new InvalidOperationException(SR.GetResourceString("AssemblyLoadContext_Verify_NotUnloading"));
             }