Fix Satellite Assembly loading (#24191)

* Fix Satellite Assembly loading

When loading satellite assemblies, we should probe next to the parent
assembly and load into the same AssemblyLoadContext as the parent
assembly.

Disable fallback probing for satellite assemblies.

Add AssemblyLoadContext.Resolving handler to probe for satellite
assemblies next to parent

Fixes #20979

* Call ResolveSatelliteAssembly from native

Only call ResolveSatelliteAssembly from native when
resolving a satellite assembly

* PR Feedback

Minimize string creation
Remove unnecessary if null checks
Eliminate corner cases by only allowing one case insensitive matching directory.

* ResolveSatelliteAssembly should ...

ResolveSatelliteAssembly should always be called on the ALC which loaded parentAssembly

Simplify code.
Add Debug.Assert

* Remove case insensitive culture search

* PR Feedback

* Fix parentAssembly logic

* Fixes from initial testing

* Add probe for lower case culture name

* PR feedback

diff --git a/src/System.Private.CoreLib/shared/System/Runtime/Loader/AssemblyLoadContext.cs b/src/System.Private.CoreLib/shared/System/Runtime/Loader/AssemblyLoadContext.cs
index c0a118c..9b7bfb4 100644 (file)
--- a/src/System.Private.CoreLib/shared/System/Runtime/Loader/AssemblyLoadContext.cs
+++ b/src/System.Private.CoreLib/shared/System/Runtime/Loader/AssemblyLoadContext.cs
@@ -187,7 +187,7 @@ namespace System.Runtime.Loader
         //
         // Inputs: The AssemblyLoadContext and AssemblyName to be loaded
         // Returns: The Loaded assembly object.
-        public event Func<AssemblyLoadContext, AssemblyName, Assembly> Resolving
+        public event Func<AssemblyLoadContext, AssemblyName, Assembly?> Resolving
         {
             add
             {
@@ -555,6 +555,43 @@ namespace System.Runtime.Loader
                 }
             }
         }
+
+        private Assembly? ResolveSatelliteAssembly(AssemblyName assemblyName)
+        {
+            // Called by native runtime when CultureName is not empty
+            Debug.Assert(assemblyName.CultureName?.Length > 0);
+
+            string satelliteSuffix = ".resources";
+
+            if (assemblyName.Name == null || !assemblyName.Name.EndsWith(satelliteSuffix, StringComparison.Ordinal))
+                return null;
+
+            string parentAssemblyName = assemblyName.Name.Substring(0, assemblyName.Name.Length - satelliteSuffix.Length);
+
+            Assembly parentAssembly = LoadFromAssemblyName(new AssemblyName(parentAssemblyName));
+
+            AssemblyLoadContext parentALC = GetLoadContext(parentAssembly)!;
+
+            string parentDirectory = Path.GetDirectoryName(parentAssembly.Location)!;
+
+            string assemblyPath = Path.Combine(parentDirectory, assemblyName.CultureName!, $"{assemblyName.Name}.dll");
+
+            if (Internal.IO.File.InternalExists(assemblyPath))
+            {
+                return parentALC.LoadFromAssemblyPath(assemblyPath);
+            }
+            else if (Path.IsCaseSensitive)
+            {
+                assemblyPath = Path.Combine(parentDirectory, assemblyName.CultureName!.ToLowerInvariant(), $"{assemblyName.Name}.dll");
+
+                if (Internal.IO.File.InternalExists(assemblyPath))
+                {
+                    return parentALC.LoadFromAssemblyPath(assemblyPath);
+                }
+            }
+
+            return null;
+        }
     }
 
     internal sealed class DefaultAssemblyLoadContext : AssemblyLoadContext

diff --git a/src/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.CoreCLR.cs b/src/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.CoreCLR.cs
index 45a2c7f..f148174 100644 (file)
--- a/src/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.CoreCLR.cs
+++ b/src/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.CoreCLR.cs
@@ -93,7 +93,7 @@ namespace System.Runtime.Loader
 
         // This method is invoked by the VM to resolve an assembly reference using the Resolving event
         // after trying assembly resolution via Load override and TPA load context without success.
-        private static Assembly ResolveUsingResolvingEvent(IntPtr gchManagedAssemblyLoadContext, AssemblyName assemblyName)
+        private static Assembly? ResolveUsingResolvingEvent(IntPtr gchManagedAssemblyLoadContext, AssemblyName assemblyName)
         {
             AssemblyLoadContext context = (AssemblyLoadContext)(GCHandle.FromIntPtr(gchManagedAssemblyLoadContext).Target)!;
 
@@ -101,6 +101,16 @@ namespace System.Runtime.Loader
             return context.ResolveUsingEvent(assemblyName);
         }
 
+        // This method is invoked by the VM to resolve a satellite assembly reference
+        // after trying assembly resolution via Load override without success.
+        private static Assembly? ResolveSatelliteAssembly(IntPtr gchManagedAssemblyLoadContext, AssemblyName assemblyName)
+        {
+            AssemblyLoadContext context = (AssemblyLoadContext)(GCHandle.FromIntPtr(gchManagedAssemblyLoadContext).Target)!;
+
+            // Invoke the ResolveSatelliteAssembly method
+            return context.ResolveSatelliteAssembly(assemblyName);
+        }
+
         private Assembly? GetFirstResolvedAssembly(AssemblyName assemblyName)
         {
             Assembly? resolvedAssembly = null;

diff --git a/src/binder/clrprivbinderassemblyloadcontext.cpp b/src/binder/clrprivbinderassemblyloadcontext.cpp
index 415b2ff..03819b8 100644 (file)
--- a/src/binder/clrprivbinderassemblyloadcontext.cpp
+++ b/src/binder/clrprivbinderassemblyloadcontext.cpp
@@ -65,9 +65,10 @@ HRESULT CLRPrivBinderAssemblyLoadContext::BindAssemblyByName(IAssemblyName     *
         //
         // 1) Lookup the assembly within the LoadContext itself. If assembly is found, use it.
         // 2) Invoke the LoadContext's Load method implementation. If assembly is found, use it.
-        // 3) Lookup the assembly within TPABinder. If assembly is found, use it.
-        // 4) Invoke the LoadContext's Resolving event. If assembly is found, use it.
-        // 5) Raise exception.
+        // 3) Lookup the assembly within TPABinder (except for satellite requests). If assembly is found, use it.
+        // 4) Invoke the LoadContext's ResolveSatelliteAssembly method (for satellite requests). If assembly is found, use it.
+        // 5) Invoke the LoadContext's Resolving event. If assembly is found, use it.
+        // 6) Raise exception.
         //
         // This approach enables a LoadContext to override assemblies that have been loaded in TPA context by loading
         // a different (or even the same!) version.

diff --git a/src/vm/appdomain.cpp b/src/vm/appdomain.cpp
index fc5d73f..480a260 100644 (file)
--- a/src/vm/appdomain.cpp
+++ b/src/vm/appdomain.cpp
@@ -6792,6 +6792,8 @@ HRESULT RuntimeInvokeHostAssemblyResolver(INT_PTR pManagedAssemblyLoadContextToB
             // Initialize the AssemblyName object from the AssemblySpec
             spec.AssemblyNameInit(&_gcRefs.oRefAssemblyName, NULL);
                 
+            bool isSatelliteAssemblyRequest = !spec.IsNeutralCulture();
+
             if (!fInvokedForTPABinder)
             {
                 // Step 2 (of CLRPrivBinderAssemblyLoadContext::BindUsingAssemblyName) - Invoke Load method
@@ -6814,9 +6816,9 @@ HRESULT RuntimeInvokeHostAssemblyResolver(INT_PTR pManagedAssemblyLoadContextToB
                 {
                     fResolvedAssembly = true;
                 }
-            
+
                 // Step 3 (of CLRPrivBinderAssemblyLoadContext::BindUsingAssemblyName)
-                if (!fResolvedAssembly)
+                if (!fResolvedAssembly && !isSatelliteAssemblyRequest)
                 {
                     // If we could not resolve the assembly using Load method, then attempt fallback with TPA Binder.
                     // Since TPA binder cannot fallback to itself, this fallback does not happen for binds within TPA binder.
@@ -6833,16 +6835,41 @@ HRESULT RuntimeInvokeHostAssemblyResolver(INT_PTR pManagedAssemblyLoadContextToB
                     }
                 }
             }
-            
-            if (!fResolvedAssembly)
+
+            if (!fResolvedAssembly && isSatelliteAssemblyRequest)
             {
                 // Step 4 (of CLRPrivBinderAssemblyLoadContext::BindUsingAssemblyName)
                 //
+                // Attempt to resolve it using the ResolveSatelliteAssembly method.
+                // Finally, setup arguments for invocation
+                BinderMethodID idHAR_ResolveSatelitteAssembly = METHOD__ASSEMBLYLOADCONTEXT__RESOLVESATELLITEASSEMBLY;
+                MethodDescCallSite methResolveSatelitteAssembly(idHAR_ResolveSatelitteAssembly);
+
+                // Setup the arguments for the call
+                ARG_SLOT args[2] =
+                {
+                    PtrToArgSlot(pManagedAssemblyLoadContextToBindWithin), // IntPtr for managed assembly load context instance
+                    ObjToArgSlot(_gcRefs.oRefAssemblyName), // AssemblyName instance
+                };
+
+                // Make the call
+                _gcRefs.oRefLoadedAssembly = (ASSEMBLYREF) methResolveSatelitteAssembly.Call_RetOBJECTREF(args);
+                if (_gcRefs.oRefLoadedAssembly != NULL)
+                {
+                    // Set the flag indicating we found the assembly
+                    fResolvedAssembly = true;
+                }
+            }
+
+            if (!fResolvedAssembly)
+            {
+                // Step 5 (of CLRPrivBinderAssemblyLoadContext::BindUsingAssemblyName)
+                //
                 // If we couldnt resolve the assembly using TPA LoadContext as well, then
                 // attempt to resolve it using the Resolving event.
                 // Finally, setup arguments for invocation
                 BinderMethodID idHAR_ResolveUsingEvent = METHOD__ASSEMBLYLOADCONTEXT__RESOLVEUSINGEVENT;
-                MethodDescCallSite methLoadAssembly(idHAR_ResolveUsingEvent);
+                MethodDescCallSite methResolveUsingEvent(idHAR_ResolveUsingEvent);
                 
                 // Setup the arguments for the call
                 ARG_SLOT args[2] =
@@ -6852,7 +6879,7 @@ HRESULT RuntimeInvokeHostAssemblyResolver(INT_PTR pManagedAssemblyLoadContextToB
                 };
 
                 // Make the call
-                _gcRefs.oRefLoadedAssembly = (ASSEMBLYREF) methLoadAssembly.Call_RetOBJECTREF(args);
+                _gcRefs.oRefLoadedAssembly = (ASSEMBLYREF) methResolveUsingEvent.Call_RetOBJECTREF(args);
                 if (_gcRefs.oRefLoadedAssembly != NULL)
                 {
                     // Set the flag indicating we found the assembly

diff --git a/src/vm/baseassemblyspec.h b/src/vm/baseassemblyspec.h
index 4fe752a..b5932f1 100644 (file)
--- a/src/vm/baseassemblyspec.h
+++ b/src/vm/baseassemblyspec.h
@@ -98,6 +98,7 @@ public:
     void SetCodeBase(LPCWSTR szCodeBase);
 
     VOID SetCulture(LPCSTR szCulture);
+    bool IsNeutralCulture();
 
     VOID ConvertPublicKeyToToken();
 

diff --git a/src/vm/baseassemblyspec.inl b/src/vm/baseassemblyspec.inl
index 5205056..9f9bae3 100644 (file)
--- a/src/vm/baseassemblyspec.inl
+++ b/src/vm/baseassemblyspec.inl
@@ -582,6 +582,11 @@ inline void BaseAssemblySpec::SetCulture(LPCSTR szCulture)
         m_context.szLocale=szCulture;
 }
 
+inline bool BaseAssemblySpec::IsNeutralCulture()
+{
+    return strcmp(m_context.szLocale,"")==0;
+}
+
 inline void BaseAssemblySpec::SetContext(ASSEMBLYMETADATA* assemblyData)
 {
     LIMITED_METHOD_CONTRACT;

diff --git a/src/vm/mscorlib.h b/src/vm/mscorlib.h
index ec96d5e..4585f8d 100644 (file)
--- a/src/vm/mscorlib.h
+++ b/src/vm/mscorlib.h
@@ -899,9 +899,10 @@ DEFINE_FIELD_U(_state,                      AssemblyLoadContextBaseObject, _stat
 DEFINE_FIELD_U(_isCollectible,              AssemblyLoadContextBaseObject, _isCollectible)
 DEFINE_CLASS(ASSEMBLYLOADCONTEXT,  Loader,                AssemblyLoadContext)
 DEFINE_METHOD(ASSEMBLYLOADCONTEXT,  RESOLVE,          Resolve,                      SM_IntPtr_AssemblyName_RetAssemblyBase)
-DEFINE_METHOD(ASSEMBLYLOADCONTEXT,  RESOLVEUNMANAGEDDLL,          ResolveUnmanagedDll,                      SM_Str_IntPtr_RetIntPtr)
+DEFINE_METHOD(ASSEMBLYLOADCONTEXT,  RESOLVEUNMANAGEDDLL,           ResolveUnmanagedDll,           SM_Str_IntPtr_RetIntPtr)
 DEFINE_METHOD(ASSEMBLYLOADCONTEXT,  RESOLVEUNMANAGEDDLLUSINGEVENT, ResolveUnmanagedDllUsingEvent, SM_Str_AssemblyBase_IntPtr_RetIntPtr)
-DEFINE_METHOD(ASSEMBLYLOADCONTEXT,  RESOLVEUSINGEVENT,          ResolveUsingResolvingEvent,                      SM_IntPtr_AssemblyName_RetAssemblyBase)
+DEFINE_METHOD(ASSEMBLYLOADCONTEXT,  RESOLVEUSINGEVENT,             ResolveUsingResolvingEvent,    SM_IntPtr_AssemblyName_RetAssemblyBase)
+DEFINE_METHOD(ASSEMBLYLOADCONTEXT,  RESOLVESATELLITEASSEMBLY,      ResolveSatelliteAssembly,      SM_IntPtr_AssemblyName_RetAssemblyBase)
 DEFINE_FIELD(ASSEMBLYLOADCONTEXT,   ASSEMBLY_LOAD,          AssemblyLoad)
 DEFINE_METHOD(ASSEMBLYLOADCONTEXT,  ON_ASSEMBLY_LOAD,       OnAssemblyLoad, SM_Assembly_RetVoid)
 DEFINE_METHOD(ASSEMBLYLOADCONTEXT,  ON_RESOURCE_RESOLVE,    OnResourceResolve, SM_Assembly_Str_RetAssembly)