Bug 60100 - AppDomain.AssemblyResolve event not triggered if Assembly already failed to load
Summary: AppDomain.AssemblyResolve event not triggered if Assembly already failed to load
Status: CONFIRMED
Alias: None
Product: Runtime
Classification: Mono
Component: General ()
Version: 5.8 (2017-10)
Hardware: Macintosh Mac OS
: Normal normal
Target Milestone: Future Cycle (TBD)
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2017-10-10 18:07 UTC by Dustin Campbell
Modified: 2018-02-12 18:17 UTC (History)
4 users (show)

Tags: bugpool-rejected
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report for Bug 60100 on GitHub or Developer Community if you have new information to add and do not yet see a matching new report.

If the latest results still closely match this report, you can use the original description:

  • Export the original title and description: GitHub Markdown or Developer Community HTML
  • Copy the title and description into the new report. Adjust them to be up-to-date if needed.
  • Add your new information.

In special cases on GitHub you might also want the comments: GitHub Markdown with public comments

Related Links:
Status:
CONFIRMED

Description Dustin Campbell 2017-10-10 18:07:37 UTC
I'm running into an where a library I'm testing hooks the AppDomain.AssemblyResolve event to locate dependencies at runtime. However, before the library's tests are executed, the test runner (xUnit) has already attempted to Assembly.LoadFrom(...) libraries from the test library's directory that require custom resolution logic before the AppDomain.AssemblyResolve event is hooked. This results in the libraries being loaded incorrectly, and once that happens, Mono will not call AppDomain.AssemblyResolve on subsequent calls to Assembly.LoadFrom(...). This works fine on .NET Framework on Windows.

If the text above doesn't make much sense, I've created a repro project at https://github.com/DustinCampbell/bug-repros/tree/master/bug1, which has more detail.
Comment 1 Ludovic Henry 2017-11-08 21:18:25 UTC
I can reproduce with Mono 5.8.0.40 (2017-10/ce494e3d152)
Comment 2 Aleksey Kliger 2018-01-10 00:05:26 UTC
Thanks for the nice repro project and writeup Dustin.

TL;DR: this seems hard.

There were 3 problems identifier in the repro.  There are actually 4.  The first three are fixable, but the 4th is a doozy.

1. On .NET touching System.Assembly.DefinedTypes throws a ReflectionTypeLoadException, while on Mono you actually have to start enumerating before you get a throw.
2. On .NET creating an instance of a class whose parent can't be resolved triggers a FileNotFoundException but a TypeLoadException on Mono.
3. On Mono you don't get a second AssemblyResolve event after an assembly has failed to load once.

Rephrased / commentary:

1. Agreed.  This is pretty easy to fix by changing the "foreach (... in GetTypes()) { yield return ... }" in System.Reflection.Assembly.get_DefinedTypes to first do a call to GetTypes() eagerly and then yield the results.

2. I think this tricky but is basically fixable.  Now that we have mono_class_set_failure () we can put whatever we like as the reason for why a class failed to load. The goal would be to set the class failure to a FNFE.  We probably rely too much on mono_class_set_type_load_failure () which is a wrapper that pretty prints a TLE message.  We should move to using mono_class_set_failure more directly.

3. I think the perception in the repro is that once we've tried and failed to load "lib1" once, we'll never fire an AssemblyResolve event for it again.  What actually happens is that once we've tried to resolve "lib2's 0th referenced assembly" (which turns out to be named "lib1") once and found it missing, we won't try to resolve lib2's 0th reference again.
To be more precise - every MonoAssembly* has an array of MonoAssemblies that it references.  The entires in the array are either MonoAssembly* or they're REFERENCE_MISSING, or they're NULL (if that reference hasn't been needed yet).  One problem is that once we set a reference as REFERENCE_MISSING, we never try again, even if there's an AssemblyResolve event handler installed in the current domain.  As soon as we see a non-NULL reference in the array, we return it.  Instead we should compare it with REFERENCE_MISSING and try resolving again (if there's a handler available, or maybe always - after all maybe the disk contents changed).  That will almost fix the repro except...

4. Once a MonoClass* has been created and marked as having a failure, it will always be marked as a failure.  We have no notion of somehow rolling the class back and trying again.
So even if we manage to find the missing assembly reference, a second call to Assembly.get_DefinedTypes will just hit the class cache return the same exact MonoClass* objects and -
 since they've already been marked as "failed" - simply return them without trying to initialize again.
In the repro, the call to DefineTypes means we try to create a MonoClass* for lib1.MyClass.  The bare minimum of what we need to initialize for that class is to get MonoClass*es for its parent and interfaces.  To get the interfaces we attempt to load the 0th referenced assembly (lib2) and since it's missing, we mark the MonoClass* for lib1.MyClass as failed.  We then put it in the class cache (so if we lookup the same typedef token again, we get the same MonoClass*).
On the second time through DefinedTypes we find the MonoClass* for MyClass in the cache and return it.  Even if the 0th referenced assembly is now found that no longer matters since we have a MonoClass* in hand and it's already marked as failed.

To change how MonoClass initialization to do something else is Really Tricky.


I'm going to keep looking, but I don't see a way around (4) right now.  One annoying bit here is that establishing a MonoClass* for the parent and each of the interfaces is basically the bare minimum of initialization that we have to do to a MonoClass.  I don't know if we can have a non-failed MonoClass* running around that doesn't even have that basic information initialized.
Comment 3 Dustin Campbell 2018-02-12 17:30:23 UTC
I see that this just got marked as "bugpool-rejected". Since this represents several issues, does that mean that all of them are rejected, or just the enormously tricky #3/#4 above?
Comment 4 Aleksey Kliger 2018-02-12 18:14:08 UTC
Dustin,

"bugpool-rejected" just means that it won't be part of Mono's monthly Bugs Week (https://github.com/mono/mono/projects/3).

I think we can work on #1-#3 but my perception is that they are relatively low-impact.
Please let me know if that perception is incorrect.
Comment 5 Dustin Campbell 2018-02-12 18:17:40 UTC
Got it. I just wasn't sure what "bugpool-rejected" meant. Thanks for clarifying!