-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
AssemblyLoadContext crash when collectible assembly use XmlSerializer #1388
Comments
Hello, Is any update on this? Thanks, |
@uffebjorklund Thanks for reporting this, sorry for the slow response. It looks like all the information we need to do further investigation is here. We will look into it. |
I did a bit of digging in to this and I believe that the problem is due to XmlSerializer creating a dynamic assembly in the default load context which makes the dynamic assembly non-collectible. When we call ILGenerator.Emit(OpCodes.Castclass, type) with a type which is a collectible assembly, we get an exception. I did some digging around the api's and I can't see a way to specify the AssemblyLoadContext to generate a dynamic assembly into. |
Hi, |
Btw this was my first project with the issue, without the XmlSerializerGenerator |
@curia-damiano How do you generate the serialization code? I tried to turning it on via VS on the Also - I noticed that your "plugins" are built as |
Hi, |
I tried again - Release build this time - and I spotted this in the output of the build:
@StephenMolloy for the XML serializer specific portions of this question, like the above failure of sgen. |
@curia-damiano, I haven't tried running your code yet but I can see a problem in Test_ALC with XmlSerializerGenerator.zip. This is the problem code: var assembly = TLC.LoadFromAssemblyPath(dllPath);
labelUnloaded.Text = "Loaded";
var type = assembly.GetType("CL.Implementation.Easy.Multiplier");
var constructor = type.GetConstructor(new Type[] { });
ICompute compute = (ICompute)constructor.Invoke(new object[] { }); When using a different AssemblyLoadContext, the same type in the same assembly is a different runtime type. So this means the |
@sdmaclea, in this comment it says it uses the callers AssemblyLoadContext. You can see here that we do use |
@mconnew Looking at the link. It looks like |
I haven't looked at the code. So I am not sure this statement above is correct. Presuming the cited code is loaded and running in For the cast to succeed:
|
Hi, @csalat: Following the comments above:
@mconnew : Also, about the cast at runtime: I get the point that there is another instance of the dll in memory and the cast doesn't work. Still I have to guess how to solve this after I am able to make the serializer generator work. My idea is that I should be able to tell the dynamic dll loader to use the Interface dll already loaded in memory. Any tip about this second point? I kindly ask all of you if you can please help me in making this simple POC to work. |
I attach here the modified POC after the changes discussed in the previous post |
@curia-damiano please consider using the same patterns as shown in this sample https://docs.microsoft.com/en-us/dotnet/core/tutorials/creating-app-with-plugin-support. It shows how to write an app with plugins where the problem with casting the interface types doesn't exist. Namely I would suggest:
In the sample app I fixed the path to the "problem" dll to use |
Hi Vitek,
Thank you very much for your extensive explanation.
Tomorrow I will go through the sample you have suggested me and adapt my
small POC accordingly.
Only, I think I still miss guidance about how to use the
XmlSerializerGenerator...
Regards, Damiano
…On Tue, Nov 26, 2019 at 4:43 PM Vitek Karas ***@***.***> wrote:
@curia-damiano <https://github.com/curia-damiano> please consider using
the same patterns as shown in this sample
https://docs.microsoft.com/en-us/dotnet/core/tutorials/creating-app-with-plugin-support.
It shows how to write an app with plugins where the problem with casting
the interface types doesn't exist.
Namely I would suggest:
- Use AssemblyDependencyResolver if possible - in the sample you
shared this is definitely a typical case where it should be used. Using
mechanisms like enumerating files in current directory is problematic (it's
not guaranteed that the current directory will be what you expect for
example, or if there's a .dll which should not be present it might get
picked up and so on).
- Build the plugins as "plugins" - that is mark the project
dependencies which should be provided by the host with the
<Private>false</Private> so that the plugins don't carry copies of
host assemblies with them.
- Unless necessary, avoid duplication of assembly resolution logic -
in your sample you implement both the AssemblyLoadContext.Load method
as well as a handler for the AssemblyLoadContext.Resolving event. You
should not need both for typical cases.
- Ideally the plugins should be self-descriptive, that is carry only
the files they need to keep in isolation and rely on the host to provide
the rest. If that is the case, then you don't need to implement any logic
to search for assemblies which are already loaded - instead, just return
null from AssemblyLoadContext.Load and the runtime will fallback to
the AssemblyLoadContext.Default which has the host (the app).
- Avoid usage of AppDomain.CurrentDomain.GetAssemblies - this returns
list of all assemblies loaded in the process, across all
AssemblyLoadContext instances. This means that if you load two plugins
which both carry some private dependency, you might end up using the
dependency from another plugin. It might work, but it would for example
prevent unloading from working correctly.
In the sample app I fixed the path to the "problem" dll to use
netcoreapp3.1 and now it fails to find the XmlSerializers.dll, which is
expected given the failure during build.
If I comment out loading the XmlSerialziers.dll the plugin itself works
correctly (so casting to ICompute works, since the custom ALC does end up
sharing the Interface assembly correctly), but it fails in the
XmlSerializer due to the wrong load context used by the dynamically
generated code.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://github.com/dotnet/corefx/issues/41286?email_source=notifications&email_token=AEN72M72A5KUZPNCNR7M2O3QVU7ZLA5CNFSM4I2BJI4KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFGOTYI#issuecomment-558688737>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEN72M45JJKS6QJJ2DIR44TQVU7ZLANCNFSM4I2BJI4A>
.
--
*Damiano Curia*
Swiss mobile: +41 76 50 40 60 2
Italian mobile: +39 348/360.25.73
skype: curia.damiano
email: [email protected]
www: http://curia.me
|
I'm not an expert in the |
I've worked out the problem with XmlSerializerGenerator. There's a temporary workaround until we work out a proper fix. The issue is being tracked in issue dotnet/corefx#40730. Unfortunately that doesn't solve this problem. |
I tried the workaround and ran into two other issues: Doesn't work with netcoreapp3.1If I set the TFM to
This in itself shows two problems:
The generated assembly is not in
|
Thank you Vitek for your further investigations. |
@vitek-karas, the set of reference assemblies that is passed to csc when compiling the generated serializer is just copied from the list of reference assemblies passed to csc when compiling your application assembly. We're not changing that list at all. If full framework assemblies are being added to the set of reference assemblies, something has changed in how the .NET Core SDK compiles that list. That's a breaking change in the .NET SDK. |
I'm not trying to put blame on anybody 😄 I'm just pointing out potential issues. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I'm having the same issue with our plugin system, a plugin may reference Is there an env-var or something that instructs |
Just encountered this as we tried to update our app to allow dynamic loading and unloading of plugins using the It appears that the fixes for this is no longer targeting .net 5 but now .net 6, am I understanding that correctly from the corresponding issues? |
While this works, it cannot be merged in due to dotnet/runtime#1388. Specifically, the Tiled plugin uses the `XmlSerializer` type, but the `XmlSerializer` tries to create dynamic assemblies in the default `AssemblyLoadContext`, which is not collectible. Therefore you have a collectible assembly creating and loading a non-collectible assembly, which is invalid and thus an exception occurs. So far this does not look like it will be fixed until .net 6 at the earliest.
While this works, it cannot be merged in due to dotnet/runtime#1388. Specifically, the Tiled plugin uses the `XmlSerializer` type, but the `XmlSerializer` tries to create dynamic assemblies in the default `AssemblyLoadContext`, which is not collectible. Therefore you have a collectible assembly creating and loading a non-collectible assembly, which is invalid and thus an exception occurs. So far this does not look like it will be fixed until .net 6 at the earliest.
Why has this bug not been fixed yet? Here is a simple example that has crashed since 2019:
|
Will the bug be fixed with .NET 6? |
@Ap0k if I understand correctly, it will be fixed in NET6; is there any chance to backport this fix to NET5? |
@lsoft Yes, the problem is only solved for NET6. Technically, these changes can be made in NET5 as well, but this is up to the repository owner. |
Thanks, @Ap0k. Looking forward for repository owner's decision. |
I am looking forward to hear if this can be fixed in Net5.0. as this is keeping our migration from .Net Framework to .Net5.0 from moving forward. |
#48072 is not in .NET 6 Preview 1. It will be in Preview 2. |
This is not fixed in .net 6... |
@chswin, this has been fixed in 7.0... and the issue unfortunately got closed when that PR was pushed through. We are trying to get approval to bring this back to 6.0, so I do expect it to show up in a servicing release. But you are correct, it is not yet fixed in .Net 6. |
The PR that brings this into the 6.0 servicing branch is #61266. |
[EDIT]
Investigation of this issue is complete and has resulted in 5 additional issues being opened.
See this comment below for links to each of those issues.
We will keep this issue open and update it as those other issues are fixed.
Please add any additional comments to those issues.
When creating a collectible AssemblyLoadContext and then use
System.Xml.Serialization.XmlSerializer
inside of a collectible assembly you will get a crash.I have a repro project: https://github.com/uffebjorklund/AssemblyLoadContextBug
Have tried a few different things
collectible = false
there is no crashXmlSerializer
code is removed from the collectible assembly there is no crashnetstandard2.0
ornetstandard2.1
on the collectible assemblyException
A non-collectible assembly may not reference a collectible assembly
Stacktrace
dotnet --info
The text was updated successfully, but these errors were encountered: