Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Xaml parser doesn't work with AssemblyLoadContext #1700

Open
vitek-karas opened this issue Aug 22, 2019 · 16 comments
Open

Xaml parser doesn't work with AssemblyLoadContext #1700

vitek-karas opened this issue Aug 22, 2019 · 16 comments
Labels
Enhancement Requested Product code improvement that does NOT require public API changes/additions rank20 Rank: Priority/rank on a scale of (1..100)
Milestone

Comments

@vitek-karas
Copy link
Member

Trying to load XAML into non-default AssemblyLoadContext can lead to very confusing errors. The underlying problem is that XAML parser is not aware of assembly load contexts. The parser itself typically runs in the Default load context, but it may be triggered to load XAML into a custom (secondary) load context. In that case all assembly resolution should happen via the secondary load context.

In order to make this easier .NET Core 3.0 introduced the "contextual reflection" concept, which can switch all reflection based APIs to use the secondary load context. More details about contextual reflection can be found in AssemblyLoadContext.ContextualReflection.md.

That fixes all cases where the XAML parser uses reflection APIs like Assembly.GetType and similar. Unfortunately the XAML parser implements its own assembly resolution logic in some cases. This logic was copied from the .NET Framework version of WPF and it still relies AppDomains, GAC and so on - it is not aware of AssemblyLoadContext. This logic can break the correct behavior: Among other things it walks all assemblies loaded into the current AppDomain (so all assemblies in the process, as .NET Core has only one AppDomain) and using custom logic resolves assembly against that list. If there are two assemblies of the same (or similar) names in that list, it will basically randomly pick the first one it finds. The code which does that is here:

internal static Assembly GetLoadedAssembly(AssemblyName assemblyName)

Assembly load contexts are typically used to implement plugin architecture. To provide good levels of isolation for each plugin, every plugin is loaded into its own load context. This can very easily lead to cases where each plugin has its own version of a certain dependency. But the above mentioned code ignores the isolation of load contexts, and will resolve assembly globally - leading to cases where the plugins will get the wrong version of dependency used.

A sample repro app is here: https://github.com/vitek-karas/WPFPluginLoadProblem
This app shows the problem with a typical plugin architecture (host app loading two plugins, each using XAML parser to load some XAML).

Originally this problem was found trying to implement tests on WPF, using ALC to provide isolation of WPF itself. The repro of that case is here: https://github.com/nick-beer/ALC-XAML-LOAD-BUG
This repro boils down to the same underlying problem.

/cc @nick-beer

@grubioe grubioe added this to the 3.0 milestone Aug 22, 2019
@vatsan-madhavan
Copy link
Member

@vitek-karas Reading through the docs etc., I'm trying to figured out the best way to enumerate loaded assemblies in an AssemblyLoadContext.

We don't want to load an Assembly if it's not already loaded for other reasons - we just want to find out if an Assembly is already loaded, and if it is, then work with it.

@weltkante
Copy link

I'm trying to figured out the best way to enumerate loaded assemblies in an AssemblyLoadContext.

Whats wrong with AssemblyLoadContext.Assemblies ? doc says "Returns a collection of the Assembly instances loaded in the AssemblyLoadContext."

we just want to find out if an Assembly is already loaded, and if it is, then work with it

The key is probably to use CurrentContextualReflectionContext or provide some other way the caller can communicate which AssemblyLoadContext to use, otherwise you would just replicate the current behavior.

@vatsan-madhavan vatsan-madhavan modified the milestones: 3.0, 3.1 Aug 27, 2019
@grubioe grubioe added Enhancement Requested Product code improvement that does NOT require public API changes/additions rank20 Rank: Priority/rank on a scale of (1..100) labels Aug 30, 2019
@vitek-karas
Copy link
Member Author

I absolutely agree with @weltkante here - the key part of the fix should be to use the right AssemblyLoadContext. How exactly is that determined depends on the actual use case, but given that we're unlikely to change the public api of the parser, using the CurrentContextualReflectionContext is probably the right way to go. Alternatively you could use just Assembly.Load which will use it.

As for enumerating assemblies, I'm really wondering why do you need to do that. As mentioned above the primary goal is to load the assembly through the right ALC. Once you have the right ALC you can just call AssemblyLoadContext.LoadFromAssemblyName. The runtime has a lookup cache very early on in this code path, so it should not be necessary for you to cache the result.

That said I noticed that the current code in WPF tries to implement somewhat custom "assembly name" -> "assembly" resolution - for example it sometimes intentionally ignores versions and so on. If we need to keep that behavior, then you would probably need to rely on assembly enumeration, in which case the Assemblies collection is the right API. This API is relatively slow, so in general I would suggest to not use it unless necessary.

Personally I'm curious why does WPF need to implement all of this custom assembly loading logic - why not use simple Assembly.Load.

Future looking: We recommend to not rely on the contextual reflection APIs, exactly because of the problem described in this issue. Almost all reflection APIs already have overloads which have some way to explicitly specify which ALC to use. For example Type.GetType has an overload which takes assembly resolver callback.
Adding something similar to the XAML parser API would be very nice.

@weltkante
Copy link

weltkante commented Sep 2, 2019

why not use simple Assembly.Load

Probably because it requires a fully qualified name including version and public key, and you don't have to specify fully qualified names in XAML. In fact most users probably don't use them (I certainly never wrote nor have seen a fully qualified assembly name in XAML ever). So removing that capability will be a breaking change to existing XAML.

@vitek-karas
Copy link
Member Author

Assembly.Load does not require fully qualified names. It accepts whatever the constructor of AssemblyName accepts, which can handle partial assembly names just fine. I tried with a simple case like this:

Loading 'System.Xml'
Success: System.Xml, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 at C:\Program Files\dotnet\shared\Microsoft.NETCore.App\3.0.0-preview8-28379-12\System.Xml.dll
Loading 'System.Xml, Version=4.0.0.0'
Success: System.Xml, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 at C:\Program Files\dotnet\shared\Microsoft.NETCore.App\3.0.0-preview8-28379-12\System.Xml.dll

In fact in .NET Core Assembly.Load(string) and Assembly.LoadWithPartialName(string) are the same (they have slightly different argument checking, but otherwise they end up calling the exact same implementation passing the name as-is).

Thanks for the link describing how WPF behaves. Roughly speaking Assembly.Load should do what WPF describes in step 1, but the devil will be in the details - it's likely that the two algorithms will not match exactly. In that case the assembly enumeration is probably the only way to go.

@weltkante
Copy link

weltkante commented Sep 2, 2019

This is probably historical, on Desktop Framework Assembly.Load didn't work unless you fully qualified the name (I keep having to work around this in our applications scripting engine regularly).

For the sake of compatibility I doubt they can move away from the established loading behavior, in particular how versions are treated in presence of loaded assemblies could differ.

If this is really a major performance problem in .NET Core they could consider switching to Assembly.Load for .NET Core 5 (or some other major version) and providing a compatibility switch to get old loading semantics. But any such optimizations should happen separately after fixing the AssemblyLoadContext problem of this issue.

@vitek-karas
Copy link
Member Author

I absolutely agree that solving the ALC problem should come first.

@mikeoliphant
Copy link

I am currently struggling with what I assume is this issue. I have a .NET 5.0 WPF plugin architecture and encounter errors if I have multiple plugins in different assembly load contexts. It seems to come down to issues with the XAML parser resolving URIs (I get odd control behavior, and "The component 'XXX' does not have a resource identified by the URI" errors.

Given that this is still an open issue, is there any known way to work around the problem? Thanks.

@wenbei421
Copy link

Have you solved your problem?

@mikeoliphant
Copy link

Have you solved your problem?

No, I have not.

@dt200r
Copy link

dt200r commented Apr 7, 2022

It seems as if there are some serious limitations to building a plug-in architecture with WPF/Core. As described above, it all appears to stem from the XAML parser's ignorance of assembly contexts. Here is a summary of our findings and resolutions.

  1. XAML namespace resolution only works if there is one instance of the referenced assembly in any assembly context (default or otherwise), even if that namespace is referring to itself (ie xmlns:local="xyz").
  2. Merged dictionary resolution (using pack uris) seems to work reliably if:
    • The referenced assembly is either in the same assembly context or the default assembly context.
    • The loading of a group of referenced assemblies into a context is atomic and includes the initialization of any resource dictionaries as part of that atomic operation (ie no simultaneous assembly loading in another thread with conflicting assembly names).

In order to come up with a workable solution, we did the following:

  1. Abandoned the use of XAML namespace references for any internal plug-in components. We only use namespaces to reference assemblies that are external to the plug-in and that are known to be present in the default assembly context of the plug-in host. As a work-around, we use ContentControls with content from merged resource dictionaries to allow code re-use amongst the plug-in assemblies.
  2. We ensure that our plug-in assemblies are atomically loaded and inititialized into a new assembly context, with no other parallel assembly loading operations happening at the same time. 'Inititialized' means that InitializeComponent is called in the ResourceDictionary. We achieve this with a partial class behind the resource dictionary XAML and a singleton accessor that creates an instance of the dictionary.

@bjoerni79
Copy link

I finally found a source where my issue has been described. Thanks to dt200r and vitek-karas, I have now a clue what is going wrong. It make sense. I have an assembly with a page and load the assembly two times (at start up and later in my code for reflection). Thanks for sharing all your thoughts on that topic.

@Camios
Copy link

Camios commented Jun 27, 2023

Curious why this is classed as a enhancement request. It looks like a regression between .NET Framework and .NET core which suggest it should be classified as a bug.

@czdietrich
Copy link

This issue should get more attention, since it makes the plugin architecture in .NET (Core) unreliable when using WPF for providing UIs and though should be treated as bug rather than as an enhancement.

@vladimir-cheverdyuk-altium

For our case it helps to set different versions of assemblies.

For example we had different assemblies controls.dll and they both had version 1.0.0.0 and they both referenced from WPF window. First loaded into default AssemblyLoadContext and second one loaded to different context. When application tries to create that WPF window using default AssemblyLoadContext and it resolves to controls.dll that loaded into non-default AssemblyLoadContext and then everything is crashing because of incompatible types.

Then I tried to increase version of first controls.dll and change reference clr-namespace:controls;assembly=controls to clr-namespace:controls;assembly=controls, Version=2.0.0.0 and it works fine. Unfortunately we have thousands of such references and changing them will be quite a big pain.

But then I found that during XAML compilation it stores version of assembly for references. For for reference clr-namespace:controls;assembly=controls it will be stored as clr-namespace:controls;assembly=controls, Version=2.0.0.0 in BAML.

Of course it will not solve original problem but perhaps it can help in some cases.

@AmeerMansour
Copy link

this issue is critical to develop addins as it allows developers to create isolated dependency for the addins to avoid conflicts to other addins and this issue since 2019 . is there any plan when it gets fixed ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Requested Product code improvement that does NOT require public API changes/additions rank20 Rank: Priority/rank on a scale of (1..100)
Projects
None yet
Development

No branches or pull requests

14 participants