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

Add GCHandle in native default ALC at creation #53308

Merged
merged 1 commit into from
Jun 5, 2021

Conversation

CoffeeFlux
Copy link
Contributor

@CoffeeFlux CoffeeFlux commented May 26, 2021

This lets embedders have a handle to fetch and pass before the runtime is properly started up and the managed default ALC is not yet created. Once the managed counterpart is initialized, the handle's target is changed, but the handle stays the same.

Fixes #48610

cc: @grendello

@ghost
Copy link

ghost commented May 26, 2021

Tagging subscribers to this area: @CoffeeFlux
See info in area-owners.md if you want to be subscribed.

Issue Details

This lets embedders have a handle to fetch and pass before the runtime is properly started up and the managed default ALC is not yet created. Once the managed counterpart is initialized, the handle's target is changed, but the handle stays the same.

cc: @grendello

Author: CoffeeFlux
Assignees: -
Labels:

area-AssemblyLoader-mono

Milestone: -

@vargaz
Copy link
Contributor

vargaz commented May 29, 2021

The failures are caused because of the code in native-library.c, which used to pass a 0 gchandle to the managed callbacks, and now it passes a non-0 one.

Adding something like:
if (mono_gchandle_get_target_internal (alc->gchandle) == NULL)
return NULL;

fixes it.

@lambdageek
Copy link
Member

Failures look related.
I think these two checks need to also check that .Target is not null not just gchALC.

return Resolve(gchALC, new AssemblyName(assemblyName));
}
// Invoked by Mono to resolve using the Resolving event after
// trying the Load override and default load context without
// success.
private static Assembly? MonoResolveUsingResolvingEvent(IntPtr gchALC, string assemblyName)
{
AssemblyLoadContext context;
// This check exists because the function can be called early in startup, before the default ALC is initialized
if (gchALC == IntPtr.Zero)
context = Default;
else
context = (AssemblyLoadContext)(GCHandle.FromIntPtr(gchALC).Target)!;
return context.ResolveUsingEvent(new AssemblyName(assemblyName));
}
// Invoked by Mono to resolve requests to load satellite assemblies.
private static Assembly? MonoResolveUsingResolveSatelliteAssembly(IntPtr gchALC, string assemblyName)
{
AssemblyLoadContext context;
// This check exists because the function can be called early in startup, before the default ALC is initialized
if (gchALC == IntPtr.Zero)
context = Default;
else
context = (AssemblyLoadContext)(GCHandle.FromIntPtr(gchALC).Target)!;
return context.ResolveSatelliteAssembly(new AssemblyName(assemblyName));

And also the call to Resolve is passing the GC Handle blindly, but it's also not expecting a null target:

AssemblyLoadContext context = (AssemblyLoadContext)(GCHandle.FromIntPtr(gchManagedAssemblyLoadContext).Target)!;

@CoffeeFlux
Copy link
Contributor Author

Yeah, I need to go through more carefully and see where we're actually using the GCHandle. There aren't that many uses, so I can just check them all.

This lets embedders have a handle to fetch and pass before the runtime is properly started up and the managed default ALC is not yet created. Once the managed counterpart is initialized, the handle's target is changed, but the handle stays the same.
@CoffeeFlux CoffeeFlux merged commit f70b5b7 into dotnet:main Jun 5, 2021
jonpryor pushed a commit to dotnet/android that referenced this pull request Jul 2, 2021
Context: https://docs.microsoft.com/dotnet/api/system.runtime.loader.assemblyloadcontext?view=net-5.0

.NET Core -- and thus, .NET 5+ -- removed most support for
`System.AppDomain` -- technically, there still exists
a single `AppDomain`, but creation of new ones is no longer possible
-- with [`System.Runtime.Loader.AssemblyLoadContext`][0] acting as
the replacement for *some* previous `AppDomain` functionality.

TL;DR: `AssemblyLoadContext` allows (potentially) loading and
unloading assemblies, but *doesn't* allow creating an in-process
"sandbox" like `AppDomain` originally did.
([Code Access Security][1] was deprecated by .NET Framework 4 and
[isn't present in .NET 5][2]; `AppDomain` for sandboxing purposes
was, in retrospect, rarely a good idea.)

Commit 0cd890b introduced partial support for using
`AssemblyLoadContext`, but it was necessarily incomplete until after
[dotnet/runtime#53308][3] and other fixes landed.

Add support for calling the new `AssemblyLoadContext`-oriented MonoVM
functions to load an assembly into either the default
`AssemblyLoadContext` (early in the startup process) or into the
application-created context later on during application run time.
MonoVM also adds new preload hooks which work with the
`AssemblyLoadContext` instead of the older AppDomains.

[0]: https://docs.microsoft.com/dotnet/api/system.runtime.loader.assemblyloadcontext?view=net-5.0
[1]: https://docs.microsoft.com/previous-versions/dotnet/framework/code-access-security/code-access-security
[2]: https://docs.microsoft.com/dotnet/core/compatibility/core-libraries/5.0/code-access-security-apis-obsolete#reason-for-change
[3]: dotnet/runtime#53308
@ghost ghost locked as resolved and limited conversation to collaborators Jul 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Mono] AssemblyLoadContext.Default must be instantiated in the runtime for embedding scenarios
3 participants