Skip to content

Commit

Permalink
Fix to set the inner exception for ALC event (dotnet#43667)
Browse files Browse the repository at this point in the history 
* Fix to set the inner exception for ALC event

Removes the exception handling at
CLRPrivBinderCoreCLR::BindAssemblyByName so that
the inner exceptioncan be set when the default
AssemblyLoadContext.Resolving handler throws

* Fixing the test for alc.default

* Enhanced the tests by customizing the exception type that
will be thrown by the handlers
  • Loading branch information
@LakshanF
LakshanF authored Oct 26, 2020
1 parent 5a4483e commit 5dfd23f
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 56 deletions.
98 changes: 47 additions & 51 deletions src/coreclr/src/binder/clrprivbindercoreclr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,76 +49,72 @@ HRESULT CLRPrivBinderCoreCLR::BindAssemblyByName(IAssemblyName *pIAssemblyNa
HRESULT hr = S_OK;
VALIDATE_ARG_RET(pIAssemblyName != nullptr && ppAssembly != nullptr);

EX_TRY
{
*ppAssembly = nullptr;
*ppAssembly = nullptr;

ReleaseHolder<BINDER_SPACE::Assembly> pCoreCLRFoundAssembly;
ReleaseHolder<AssemblyName> pAssemblyName;
ReleaseHolder<BINDER_SPACE::Assembly> pCoreCLRFoundAssembly;
ReleaseHolder<AssemblyName> pAssemblyName;

SAFE_NEW(pAssemblyName, AssemblyName);
IF_FAIL_GO(pAssemblyName->Init(pIAssemblyName));
SAFE_NEW(pAssemblyName, AssemblyName);
IF_FAIL_GO(pAssemblyName->Init(pIAssemblyName));

hr = BindAssemblyByNameWorker(pAssemblyName, &pCoreCLRFoundAssembly, false /* excludeAppPaths */);
hr = BindAssemblyByNameWorker(pAssemblyName, &pCoreCLRFoundAssembly, false /* excludeAppPaths */);

#if !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE)
if ((hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND)) ||
(hr == FUSION_E_APP_DOMAIN_LOCKED) || (hr == FUSION_E_REF_DEF_MISMATCH))
if ((hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND)) ||
(hr == FUSION_E_APP_DOMAIN_LOCKED) || (hr == FUSION_E_REF_DEF_MISMATCH))
{
// If we are here, one of the following is possible:
//
// 1) The assembly has not been found in the current binder's application context (i.e. it has not already been loaded), OR
// 2) An assembly with the same simple name was already loaded in the context of the current binder but we ran into a Ref/Def
// mismatch (either due to version difference or strong-name difference).
//
// Attempt to resolve the assembly via managed ALC instance. This can either fail the bind or return reference to an existing
// assembly that has been loaded
INT_PTR pManagedAssemblyLoadContext = GetManagedAssemblyLoadContext();
if (pManagedAssemblyLoadContext == NULL)
{
// If we are here, one of the following is possible:
//
// 1) The assembly has not been found in the current binder's application context (i.e. it has not already been loaded), OR
// 2) An assembly with the same simple name was already loaded in the context of the current binder but we ran into a Ref/Def
// mismatch (either due to version difference or strong-name difference).
//
// Attempt to resolve the assembly via managed ALC instance. This can either fail the bind or return reference to an existing
// assembly that has been loaded
INT_PTR pManagedAssemblyLoadContext = GetManagedAssemblyLoadContext();
if (pManagedAssemblyLoadContext == NULL)
// For satellite assemblies, the managed ALC has additional resolution logic (defined by the runtime) which
// should be run even if the managed default ALC has not yet been used. (For non-satellite assemblies, any
// additional logic comes through a user-defined event handler which would have initialized the managed ALC,
// so if the managed ALC is not set yet, there is no additional logic to run)
SString &culture = pAssemblyName->GetCulture();
if (!culture.IsEmpty() && !culture.EqualsCaseInsensitive(g_BinderVariables->cultureNeutral))
{
// For satellite assemblies, the managed ALC has additional resolution logic (defined by the runtime) which
// should be run even if the managed default ALC has not yet been used. (For non-satellite assemblies, any
// additional logic comes through a user-defined event handler which would have initialized the managed ALC,
// so if the managed ALC is not set yet, there is no additional logic to run)
SString &culture = pAssemblyName->GetCulture();
if (!culture.IsEmpty() && !culture.EqualsCaseInsensitive(g_BinderVariables->cultureNeutral))
{
// Make sure the managed default ALC is initialized.
GCX_COOP();
PREPARE_NONVIRTUAL_CALLSITE(METHOD__ASSEMBLYLOADCONTEXT__INITIALIZE_DEFAULT_CONTEXT);
DECLARE_ARGHOLDER_ARRAY(args, 0);
CALL_MANAGED_METHOD_NORET(args)

pManagedAssemblyLoadContext = GetManagedAssemblyLoadContext();
_ASSERTE(pManagedAssemblyLoadContext != NULL);
}
// Make sure the managed default ALC is initialized.
GCX_COOP();
PREPARE_NONVIRTUAL_CALLSITE(METHOD__ASSEMBLYLOADCONTEXT__INITIALIZE_DEFAULT_CONTEXT);
DECLARE_ARGHOLDER_ARRAY(args, 0);
CALL_MANAGED_METHOD_NORET(args)

pManagedAssemblyLoadContext = GetManagedAssemblyLoadContext();
_ASSERTE(pManagedAssemblyLoadContext != NULL);
}
}

if (pManagedAssemblyLoadContext != NULL)
if (pManagedAssemblyLoadContext != NULL)
{
hr = AssemblyBinder::BindUsingHostAssemblyResolver(pManagedAssemblyLoadContext, pAssemblyName, pIAssemblyName,
NULL, &pCoreCLRFoundAssembly);
if (SUCCEEDED(hr))
{
hr = AssemblyBinder::BindUsingHostAssemblyResolver(pManagedAssemblyLoadContext, pAssemblyName, pIAssemblyName,
NULL, &pCoreCLRFoundAssembly);
if (SUCCEEDED(hr))
// We maybe returned an assembly that was bound to a different AssemblyLoadContext instance.
// In such a case, we will not overwrite the binding context (which would be wrong since it would not
// be present in the cache of the current binding context).
if (pCoreCLRFoundAssembly->GetBinder() == NULL)
{
// We maybe returned an assembly that was bound to a different AssemblyLoadContext instance.
// In such a case, we will not overwrite the binding context (which would be wrong since it would not
// be present in the cache of the current binding context).
if (pCoreCLRFoundAssembly->GetBinder() == NULL)
{
pCoreCLRFoundAssembly->SetBinder(this);
}
pCoreCLRFoundAssembly->SetBinder(this);
}
}
}
}
#endif // !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE)

IF_FAIL_GO(hr);
IF_FAIL_GO(hr);

*ppAssembly = pCoreCLRFoundAssembly.Extract();
*ppAssembly = pCoreCLRFoundAssembly.Extract();

Exit:;
}
EX_CATCH_HRESULT(hr);

return hr;
}
Expand Down
12 changes: 10 additions & 2 deletions src/tests/Loader/binding/tracing/BinderTracingTest.EventHandlers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@

namespace BinderTracingTests
{
public class BinderTestException : Exception
{
public BinderTestException(string message)
: base(message)
{
}

}
partial class BinderTracingTest
{
private const string AssemblyLoadFromHandlerName = "LoadFromResolveHandler";
Expand Down Expand Up @@ -381,7 +389,7 @@ public void Dispose()
private Assembly OnAssemblyLoadContextResolving(AssemblyLoadContext context, AssemblyName assemblyName)
{
if (handlerReturn == HandlerReturn.Exception)
throw new Exception("Exception in handler for AssemblyLoadContext.Resolving");
throw new BinderTestException("Exception in handler for AssemblyLoadContext.Resolving");

Assembly asm = ResolveAssembly(context, assemblyName);
var invocation = new HandlerInvocation()
Expand All @@ -403,7 +411,7 @@ private Assembly OnAssemblyLoadContextResolving(AssemblyLoadContext context, Ass
private Assembly OnAppDomainAssemblyResolve(object sender, ResolveEventArgs args)
{
if (handlerReturn == HandlerReturn.Exception)
throw new Exception("Exception in handler for AppDomain.AssemblyResolve");
throw new BinderTestException("Exception in handler for AppDomain.AssemblyResolve");

var assemblyName = new AssemblyName(args.Name);
var customContext = new CustomALC(nameof(OnAppDomainAssemblyResolve));
Expand Down
6 changes: 3 additions & 3 deletions src/tests/Loader/binding/tracing/BinderTracingTest.ResolutionFlow.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ public static BindOperation AssemblyLoadContextResolvingEvent_CustomALC_Exceptio
CustomALC alc = new CustomALC(nameof(AssemblyLoadContextResolvingEvent_CustomALC_Exception));
using (var handlers = new Handlers(HandlerReturn.Exception, alc))
{
Assert.Throws<FileLoadException, Exception>(() => alc.LoadFromAssemblyName(assemblyName));
Assert.Throws<FileLoadException, BinderTestException>(() => alc.LoadFromAssemblyName(assemblyName));

return new BindOperation()
{
Expand Down Expand Up @@ -444,7 +444,7 @@ public static BindOperation AssemblyLoadContextResolvingEvent_DefaultALC_Excepti
var assemblyName = new AssemblyName(SubdirectoryAssemblyName);
using (var handlers = new Handlers(HandlerReturn.Exception, AssemblyLoadContext.Default))
{
Assert.Throws<FileLoadException>(() => AssemblyLoadContext.Default.LoadFromAssemblyName(assemblyName));
Assert.Throws<FileLoadException, BinderTestException>(() => AssemblyLoadContext.Default.LoadFromAssemblyName(assemblyName));

return new BindOperation()
{
Expand Down Expand Up @@ -551,7 +551,7 @@ public static BindOperation AppDomainAssemblyResolveEvent_Exception()
CustomALC alc = new CustomALC(nameof(AppDomainAssemblyResolveEvent_Exception));
using (var handlers = new Handlers(HandlerReturn.Exception))
{
Assert.Throws<FileLoadException, Exception>(() => alc.LoadFromAssemblyName(assemblyName));
Assert.Throws<FileLoadException, BinderTestException>(() => alc.LoadFromAssemblyName(assemblyName));

return new BindOperation()
{
Expand Down

0 comments on commit 5dfd23f

Please sign in to comment.