From 69e8aaa758a6322224a01f9169ead3f5c194f155 Mon Sep 17 00:00:00 2001 From: Steve MacLean Date: Mon, 29 Apr 2019 15:37:28 -0400 Subject: [PATCH] Fix Satellite Assembly loading (dotnet/coreclr#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 dotnet/coreclr#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 Commit migrated from https://github.com/dotnet/coreclr/commit/a65e2b9f1fe36d9b4dd0c850ec71984919547e18 --- .../Runtime/Loader/AssemblyLoadContext.CoreCLR.cs | 12 ++++++- .../binder/clrprivbinderassemblyloadcontext.cpp | 7 ++-- src/coreclr/src/vm/appdomain.cpp | 39 ++++++++++++++++++---- src/coreclr/src/vm/baseassemblyspec.h | 1 + src/coreclr/src/vm/baseassemblyspec.inl | 5 +++ src/coreclr/src/vm/mscorlib.h | 5 +-- .../System/Runtime/Loader/AssemblyLoadContext.cs | 39 +++++++++++++++++++++- 7 files changed, 95 insertions(+), 13 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.CoreCLR.cs index 45a2c7f..f148174 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.CoreCLR.cs +++ b/src/coreclr/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/coreclr/src/binder/clrprivbinderassemblyloadcontext.cpp b/src/coreclr/src/binder/clrprivbinderassemblyloadcontext.cpp index 415b2ff..03819b8 100644 --- a/src/coreclr/src/binder/clrprivbinderassemblyloadcontext.cpp +++ b/src/coreclr/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/coreclr/src/vm/appdomain.cpp b/src/coreclr/src/vm/appdomain.cpp index fc5d73f..480a260 100644 --- a/src/coreclr/src/vm/appdomain.cpp +++ b/src/coreclr/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/coreclr/src/vm/baseassemblyspec.h b/src/coreclr/src/vm/baseassemblyspec.h index 4fe752a..b5932f1 100644 --- a/src/coreclr/src/vm/baseassemblyspec.h +++ b/src/coreclr/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/coreclr/src/vm/baseassemblyspec.inl b/src/coreclr/src/vm/baseassemblyspec.inl index 5205056..9f9bae3 100644 --- a/src/coreclr/src/vm/baseassemblyspec.inl +++ b/src/coreclr/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/coreclr/src/vm/mscorlib.h b/src/coreclr/src/vm/mscorlib.h index ec96d5e..4585f8d 100644 --- a/src/coreclr/src/vm/mscorlib.h +++ b/src/coreclr/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) 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 c0a118c..9b7bfb4 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 @@ -187,7 +187,7 @@ namespace System.Runtime.Loader // // Inputs: The AssemblyLoadContext and AssemblyName to be loaded // Returns: The Loaded assembly object. - public event Func Resolving + public event Func 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 -- 2.7.4