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

Mixed assembly support #207

Closed
gureedo opened this issue Nov 15, 2023 · 10 comments · Fixed by #209
Closed

Mixed assembly support #207

gureedo opened this issue Nov 15, 2023 · 10 comments · Fixed by #209

Comments

@gureedo
Copy link

gureedo commented Nov 15, 2023

Hi!

Is it possible to return mixed assembly support?
Latest version is unable to perform tlbexport for mixed assembly or assembly with mixed assembly dependency.
I have digged a bit and found that AssemblyLoadContext is constructed with unload feature which is not supported for mixed assemblies.

@SOsterbrink
Copy link
Member

Hi @gureedo ,

what kind of mixed assemblies are you talking about? 32-bit and 64-bit assemblies? Different .NET versions? C# and c++ ?
64-bit assemblies which call COM interfaces of 32-bit assemblies or the other way around?

Can you provide an example for what you want to do?

@gureedo
Copy link
Author

gureedo commented Nov 15, 2023

Hi!
This one https://learn.microsoft.com/en-us/cpp/dotnet/mixed-native-and-managed-assemblies?view=msvc-170
I have a lot of managed assemblies that depends on mixed assemblies.

@SOsterbrink
Copy link
Member

Which version worked and is it only the newest version which is affected?

I'll have to take a closer look how the previous fix or the dependency updates introduced the problem.

@gureedo
Copy link
Author

gureedo commented Nov 15, 2023

Current behavior was introduced in #192
And I have found why it isn't allowed to unload mixed assemblies dotnet/coreclr#23430

@SOsterbrink
Copy link
Member

Thank you, that's very helpful. I'll take a closer look what I can do about it and talk to @mlessmann who wrote the relevant change. The problem which motivated the change is sadly not documented here in GitHub.

But it's likely that this will take some days to resolve.

@marklechtermann
Copy link
Member

@SOsterbrink

Maybe we could check in the Dispose method whether it is a mixed assembly.
In this case, we could ignore to call OnLoad method.

Mybe with...
https://learn.microsoft.com/en-us/dotnet/api/system.reflection.module.getpekind?view=net-7.0

Module.GetPEKind(PortableExecutableKinds, ImageFileMachine)

stack overflow to the rescue...
https://stackoverflow.com/a/17626536/8690889

Unfortunately, I'm not really familiar with mixed assemblies.

@SOsterbrink
Copy link
Member

I'm not using mixed assemblies either, so we will have to find an example to test things.

My first idea was to introduce a mixedAssemblyMode command line switch. If this is set, the IsCollectible property of the AssemblyLoadContext is false and the unload step at the end is skipped. That should produce the old behavior.

But I'll also take a closer look at your suggestion tomorrow. because having to manually specify the option can get tedious.

SOsterbrink added a commit that referenced this issue Nov 15, 2023
#207
We will have to evaluate if this fixes the problem
@SOsterbrink
Copy link
Member

After further study of the problem: Your idea will not work @marklechtermann because to use Module.GetPEKind we would have to load the assembly to determine it's PEKind. In the example and the documentation I don't see a way to use it without first loading the individual assembly.

Also the AssemblyLoadContext we use here is a global thing for the dscom and we would have to determine this information before loading the first assembly into this load context. The information about whether to context allows unloading assemblies needs to be know during the Creator call.

So the only way I see to do it is to select the desired behavior before creating the AssemblyLoadContext and the easiest way would be a manually set option.

@marklechtermann
Copy link
Member

Brief explanation of the patch:

We (@SOsterbrink and @matthiasnissen) discussed this today.
In the case of the command line client, it should not be necessary to unload the assemblies.
Since the process ends after generation anyway.

marklechtermann pushed a commit that referenced this issue Nov 16, 2023
* fix: Set AssemblyLoadContext to be not unload-able #207
* fix: removed call to unload the assembly
@marklechtermann
Copy link
Member

Fixed with Release v1.5.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants