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

Override assembly names loaded through AssemblyResolve handlers when searching currently loaded assemblies. #48

Merged
merged 2 commits into from
Sep 30, 2023

Conversation

vitorhnn
Copy link

The program that led me down this rabbit hole is STORY OF SEASONS: A Wonderful Life's launcher. (steam appid 2111170).

It uses an AssemblyResolve handler to load assemblies from its own resources, but it ships a version of System.Threading.Tasks.Extensions that doesn't match the version requested by its assemblies.

Framework handles this by assuming whatever's returned from AssemblyResolve handlers is the requested assembly and caching that for future loads.

Mono doesn't do this and double loads this assembly (due to two of the launcher's dependencies triggering a load), which leads to a failure to setup a vtable due to mismatching types (they're the same type, but loaded from two different instances of the same assembly).

I have a more realistic repro of the issue here. It should build fine by just using msbuild

Not entirely happy with this solution, but the bits of the assembly loader that insert the assembly into MonoDomain::domain_assemblies do not receive the name it was requested under, and passing that in would entail changing parts of Mono's assembly load hooks, which I wasn't comfortable with. Let me know if that would be preferrable and I can try to go for it.

@madewokherd
Copy link
Owner

Other than the data structure used, I think this is the right approach.

@vitorhnn vitorhnn force-pushed the develop branch 2 times, most recently from dcb0d68 to 6abada0 Compare September 18, 2023 21:12
@vitorhnn
Copy link
Author

v2: Switched to a HashTable of pointer arrays to deal with assemblies potentially having multiple aliases.

@madewokherd
Copy link
Owner

I don't really understand the advantage of a hash table at all, given that we must visit all entries when using it. I feel like a list of assembly/name pairs would be more efficient.

@vitorhnn
Copy link
Author

Hmm, do you mean something like this?:

mono_domain_assembly_search(...)
{
    ...
    mono_domain_assemblies_lock(domain);
    for (tmp = domain->overriden_names; tmp; tmp->next)
        /* try to match with tmp->aname */
    for (tmp = domain->domain_assemblies; tmp; tmp->next)
        /* same code as current develop */
    mono_domain_assemblies_unlock(domain);
    ...
}

or something more intrusive, like changing domain->domain_assemblies to be a list of aname, assembly pairs?

@madewokherd
Copy link
Owner

Yep, something like that.

… an AssemblyResolve handler.

In some cases. Namely during the assembly search process; enumerating assemblies
through AppDomain.GetAssemblies will still return the proper assembly name
@vitorhnn
Copy link
Author

v3: Ditched the hash table

@madewokherd
Copy link
Owner

Looks good to me, I'll merge it in if the CI doesn't show any problems. (Note: CI will probably fail, don't worry about it, I'll let you know if any of it looks related.)

@madewokherd madewokherd merged commit 6e566a6 into madewokherd:develop Sep 30, 2023
2 of 4 checks passed
@madewokherd
Copy link
Owner

This was released in Wine Mono 8.1.0, which I just pushed to Proton Experimental bleeding-edge.

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 this pull request may close these issues.

2 participants