Dynamic component loading draft

[vitek-karas]

Very first draft of the "plugin" design.

TODOs:

• Short description of .deps.json functionality - to demonstrate the capabilities we'll enable by using it.
• Section about expected tooling experiences - what does it mean to build a "plugin" using .NET SDK and so on.
• Section about settings/config knobs - env. variables, roll forward settings, command line arguments - which will have effect of component loading and which will be ignored and how.
• Section about interactions of the new ALC with existing extension points of the binder (various events on Assembly, AppDomain and AssemblyLoadContext, interation with other ALCs, native asset resolution extension points ...)
• More details on native asset resolution
• More details on satellite assembly resolution
• Handling of NI images

[jkotas]

Looks good to me overall

[jkotas]

those find -> those found

[jkotas]

I do not think that Assembly.LoadFrom should be mentioned in the first bullet. It is mentioned in the second bullet already.

[vitek-karas]

I was thinking about it... Assembly.Load only loads by assembly name, Assembly.LoadFrom can do the same but given a file path. But I'll remove it for simplicity.

[jkotas]

I would describe the API shape in the standard "API review" format, like:

class Assembly
{
    public static Assembly LoadFileWithDependencies(string path);
}

[jeffschwMSFT]

For unification purposes, I would recommend that this hang off of AssemblyLoadContext. The universe of Assembly.Load* and AppDomain.* have legacy behaviors that may not be intuitive. From a higher level strategy I think people should direct people towards ALCs.

[jkotas]

Add the proposed API with the unloading flag.

[jkotas]

Nit: "High-level description of the solution" -> `High-level description of the implementation" ?

[jeffschwMSFT]

I think the following areas are interesting to include in this discussion:

• What other work is dependent/related to this: unifying pub and bin, unifying dotnet-cli and VS working directory on run, unloadability, etc.
• I think we need to step back and put out a proposal of ALC and what all we plan on doing it together. We are adding individual pieces here and there, but I feel we need a bit more unified evolution.
• What is outstanding from this proposal that should be addressed in later ones?
  Those are my high level thoughts for now.

[jeffschwMSFT]

For unification purposes, I would recommend that this hang off of AssemblyLoadContext. The universe of Assembly.Load* and AppDomain.* have legacy behaviors that may not be intuitive. From a higher level strategy I think people should direct people towards ALCs.

[jkotas]

From a higher level strategy I think people should direct people towards ALCs.

The expert users have been directed towards ALC already.

I think there is a benefit in having the simple mainstream API to not be tangled with ALC.

[jeffschwMSFT]

I think there is a benefit in having the simple mainstream API to not be tangled with ALC.

In that case, I think we need to reconcile the behaviors of the other AppDomain.* to ensure they all play nicely together.

[jkotas]

reconcile the behaviors of the other AppDomain.* to ensure they all play nicely together.

Example what do you have in mind? The existing Assembly.Load* APIs have a well defined behavior that should not be affected by introducing this new thing.

[jeffschwMSFT]

We added behaviors to AssemblyResolve event, etc. to act just like desktop. Do we want to carry those behaviors along with this new model? If you do an AppDomain.LoadFrom the default assembly resolve event will kick in and try to resolve a failed attempt. How will that play with the new ALC resolve event that handles the deps.json.

[jkotas]

Do we want to carry those behaviors along with this new model?

Yes, I do not see why we would break the existing APIs and change them to behave differently. We may not like how these APIs behave, but that is not a good reason for breaking them.

Assembly.LoadFrom does what it does. It is unpredictable API in .NET Framework. It is more predictable best-effort implementation in .NET Core. It rarely does (in either .NET Core or .NET Framework) what people expect it to do. (Nit: It is Assembly.LoadFrom not AppDomain.LoadFrom.)

[vitek-karas]

Trying to answer @jeffschwMSFT questions:

What other work is dependent/related to this: unifying pub and bin, unifying dotnet-cli and VS working directory on run, unloadability, etc.

I think this is independent. If this new ALC takes into consideration the .runtimeconfig.dev.json then it should be able to load components produced with both dotnet build and dotnet publish. It should also not depend on current directory in any way (at least as far as I can tell). There's a larger issue of how it interacts with broader environment (env. variables, command line options, app's .runtimeconfig.json and so on) - this is currently a TODO for this doc (see the PR description).

Unloadability is already mentioned here, and I added a new API overload which would directly enable it on the new ALC.

I think we need to step back and put out a proposal of ALC and what all we plan on doing it together. We are adding individual pieces here and there, but I feel we need a bit more unified evolution.

Here I also think that these are almost entirely independent. Improving experience for people trying to implement their own ALC is almost completely orthogonal to us providing a stock implementation of an ALC.
I added a new public API overload which allows specifying fallback (parent) ALC, which might be of use for the more advanced scenarios (dotnet watch using this ALC to load the app, which will in turn try to load a plugin would probably need this).

Similar to the other discussion in this PR - this proposal is to provide a solution to a problem for people to use without them needing to know much about ALCs or us forcing them to go learn ALCs.

What is outstanding from this proposal that should be addressed in later ones?

In this PR description there's a list of TODOs for things I know we need to add to this document. On top of that I only think the usual applies (so I didn't list it anywhere):

• detailed technical design
• review from interested parties
• test plan
• docs
• ….

[vitek-karas]

Regarding new API on Assembly versus AssemblyLoadContext

Given the feedback we've got so far on various Assembly.Load* methods and custom ALC implementations I think the main goal of this feature should be:

Add a simple to use mechanism to dynamically load a component with reasonable defaults

To me this also comes with

Don't force people onto AssemblyLoadContext for a typical use case

That's what the Assembly.LoadFileWithDependencies API is for. For people which just want sensible defaults and follow our golden paths (regarding tooling and deployment and so on). This set of users should not need to know anything about ALC.

The APIs on AssemblyLoadContext are for the more advanced scenarios, where it's reasonable to assume at least a high-level idea about ALC, isolation and so on.

Regarding interaction with existing extension points

I added an item to the TODO list to go into more details about this. But in general I agree with @jkotas, I don't see a reason to treat this new ALC in any special way. That said I'm sure I'm missing a lot of things, so if you have a specific case in mind, please let me know.

[swaroop-sridhar]

For the TODO of "Short description of .deps.json functionality," how about

Abilities of deps.json
deps.json is a dependency manifest for dotnet apps. It enables plugins to load dependencies from locations other than the base directory (ex: from packages, platform-specific publish directories, etc.)

CoreCLR first builds a set of directories to look for binaries (called ProbingPaths ) based on application/plugin base directory, framework directory, runtimeconfig.dev.json, command line, servicing locations, etc.

For an application or plugin, the deps.json file specifies:

• A set of binary dependencies -- which are searched within ProbingPaths
• Relative paths to locate them -- Relative paths within deps.json file can be used to locate architecture-specific dependencies within publish/package directories.
  If the app depends on one or more frameworks, the deps.json files of those framework are similarly processed.

Further details about the algorithm used for processing dependencies can be found at:
https://github.com/dotnet/core-setup/blob/master/Documentation/design-docs/assembly-conflict-resolution.md

[vitek-karas]

Thanks a lot @swaroop-sridhar - I added it into the document (only slightly modified to fir the structure).

[sdmaclea]

Nit change 'amount' to 'set'

[sdmaclea]

Is this a strict requirement for all frameworks? With the possibility of multiple frameworks this seems like an unnecessary restriction. It might just work. Is this just for restricting the set of supported and tested features?

Perhaps it is reasonable to defer this until it becomes an issue.

[vitek-karas]

There are some small issues technically, but nothing major. This is more in line of what we want to support for now. The problem is that frameworks might not expect to exist in multiple copies (for example WPF is likely to have issues if two copies are running in the same process). There's also the problem of sharing. Reasonable expectation for frameworks is that they are "shared" by all components in the app. That would require us to load the framework into the default context (even when resolving it for the component) - which comes with lot of tricky issues.

[sdmaclea]

Can we think about plug-in versioning and app compatibility. Is there a reasonable need to provide a standardized mechanism for plugin to express app requirements. This might be as simple as adding a deps.json section with convenience API to read it.

If I was going to create plug-in picker, I would only want to show compatible plug-ins.

[vitek-karas]

There are things arleady in place which might help with this. For example:

• The plugin is likely to have a dependency on a an assembly from the app (the one which defines the interface they're going to talk through).
• This dependency is in the .deps.json and it has a version - the version which the plugin was compiled with.
• There's existing managed API to read .deps.json including this information.

It's not exactly first class support, but at least something is there.

I'll add it to the list of possible improvements to add something more specific.

[sdmaclea]

The plugin is likely to have a dependency on a an assembly from the app

Since it will be provided by the app it will not typically be needed in the plugins deps.json. So it might want to be declared specially. Something like

"pluginAppDependencies" : [ "MyAppPluginModel-3.3.x" ]

[vitek-karas]

Currently even if a dependency is set to not be included in the output, it will still be listed in the .deps.json. Depending on a situation it might be just a declaration like

"Dependency/2.0.1" {},

Or it may actually list the assets as well (would contain the actuall .dll path)
But I agree that this doesn't feel like first class support.

[sdmaclea]

LGTM

[swaroop-sridhar]

@jeffschwMSFT asked about loading assemblies (and their dependencies) to an existing load-context, typically the default load context. If this is a typical use case for developers, we should support it, via an isolation switch.

class Assembly
{
    public static Assembly LoadFileWithDependencies(string path, bool Isolated);
}

I don't know of any technical reasons not to support this behavior.

[jkotas]

How would this API handle conflicts? Fail right away if there are any conflicts? Do we have a real app example that would use this API?

[jeffschwMSFT]

80% of existing use of ALCs load within the default. I think we should consider this as an append to the end scenario. If there is a conflict and the existing TPA cannot resolve it, then it would fail to load. We could either do that proactively or fail at the point of the load of the dependent assembly.

[vitek-karas]

I'm just curious: How did we learn that 80% of ALC use cases are loading into default?
Also - what do you mean by that: That in the Load the ALC's code calls something like Assembly.LoadFrom?
Also - do we know if that's the intended behavior people wanted?

Do you have a sample I can look at - I would be really interested why would somebody do this...

[jkotas]

80% of existing use of ALCs load within the default.

Do you have a real world example that would be a good candidate for using LoadFileWithDependencies into default context API?

[jeffschwMSFT]

WRT customer feedback. I hope there are multiple avenues to get early feedback: let's move this discussion to coreclr and invite people with existing feedback to chime in, talk with Richard to get early enterprise'ish usage, and get them into preview for feedback.

[jeffschwMSFT]

As it stands today, the only way to communicate between contexts is via reflection with only runtime types that exchange.

This is not the case. The component can choose to share assembly with the app, by not including it in its deployment.

Perhaps we are having a difference in words only... but for me CreateInstance is not a natural pattern and fits within my reflection use case. If we had a LoadAndUnwrap then perhaps that is pretty natural, as that does not require any prior knowledge of what is loaded where and you get an 'object' back.

[AaronRobinsonMSFT]

My 2 cents here is that isolation should be the default since that seems to be the entire point of this feature - but perhaps not on this API. I am generally leery of APIs that take bool arguments because any API that takes a bool is invariably trying to make a decision about what the default behavior. In this case adding a new function that indicates Assembly.LoadIsolated() makes way more sense to me and is clear about the very new semantics we are providing.

[jeffschwMSFT]

+1 on this API in essence being the 'append to an existing ALC' by loading with dependencies and the ALC API being the way to load with isolation. The more general scenario that I hope we are able to enable is loading multiple plug-ins into the same ALC (be it either default or a new one). Having a 1:1 relationship of a deps to a ALC (which is what isolation by default in essence turns into) I feel is too limiting.

[jkotas]

I am wondering whether it would be better to drop the simple convenience API from the first phase given that we do not have clear answer for the right default. We can always introduce it later once we have better data about what the default should be and we have somebody asking for it.

[jkotas]

We are not using the .ni.dll suffix for native images. I think it was always the case in .NET Core. There are historic places that still use it, but I do not think we are shipping with the .ni.dll extension anywhere.

The problem with using .ni.dll suffix for native images is that everything has to be aware of it and strip/add it as necessary.

[vitek-karas]

I was looking into the runtime. The TPA binder will look for .ni.dll, .ni.exe and .ni.winmd and prioritize those over non-NI paths. Also looking into AssemblyBinder::GetAssembly it does make a lot of different decisions if the assembly is NI or not. And as far as I can tell, it only determines that by the fExplicitBindToNativeImage parameter which is set to TRUE based on the file extensions as mentioned above.

I admit I haven't debugged through it to see exactly what's going on... but it looks different to what you describe. But maybe I'm just reading the code in the wrong way.

[jkotas]

I agree that there are places that handle the .ni.dll extension. I guess I am trying to say that those paths are not exercised and likely buggy because of we are not shipping binaries with .ni.dll extension.

[vitek-karas]

Got it - that simplifies things.

[swaroop-sridhar]

We should mention that if the same R2R image is loaded by two ALCs, only one of them can use the native code. This is not a limitation of the plugin model per say, but we should mention it in the context.

[AaronRobinsonMSFT]

collide with anything in the app itself

This is an ambiguous statement. Unsure about how to make it more precise, but what this means is definitely different based on the experience of the person answering a question about Assembly.LoadFrom().

[vitek-karas]

This section is a very short description of existing APIs, it's not meant to be 100% precise. If you can point out what you mean - I honestly don't see anything wrong with the sentence...

[AaronRobinsonMSFT]

In lot of these cases

"In most of these cases"

[AaronRobinsonMSFT]

Please include a link to @janvorli 's assembly unload document.

[vitek-karas]

No such document exists - yet.

[AaronRobinsonMSFT]

My 2 cents here is that isolation should be the default since that seems to be the entire point of this feature - but perhaps not on this API. I am generally leery of APIs that take bool arguments because any API that takes a bool is invariably trying to make a decision about what the default behavior. In this case adding a new function that indicates Assembly.LoadIsolated() makes way more sense to me and is clear about the very new semantics we are providing.

[AaronRobinsonMSFT]

In its core

"At its core"

[AaronRobinsonMSFT]

like Assembly.LoadFile it provides isolation, but also for the dependencies.

That already provides isolation? I thought that is what we are providing here.

[vitek-karas]

Assembly.LoadFile already loads the specified assembly in isolation. But it's very simplistic. It always creates a new ALC every time its called and loads the specified file into it. It doesn't provide any resolution logic and falls back to default for everything. So the loaded assembly itself is fully isolated, but all its dependencies will be resolved from Default.

That's why the new proposed API (if we go with isolation by default) is closest in behavior for Assembly.LoadFile (and thus the name as well)

[jkotas]

It always creates a new ALC every time its called

Nit: If you call it twice with the same argument, only one ALC gets created. It creates a new ALC per absolute assembly path, not every time it's called.

[AaronRobinsonMSFT]

I see little value for the plain string argument. We should have a binary path for this kind of thing either (a) the caller wants predictable and well-defined behavior or (b) the caller has some special scenario and they need to do some work. I think we are providing too many entry points for an already niche scenario in the grand scheme of things. We can always add and make simpler, but removing is impossible.

[vitek-karas]

Makes sense

[AaronRobinsonMSFT]

Can you describe the "host" components? Does custom hosting require providing an implementation of this mechanism?

[swaroop-sridhar]

I guess the host-components here is the code in hostpolicy.dll that does deps.json parsing. If a custom host should call LoadWithDependencies then it must provide an implementation of this functionality. We should consider whether it is worth extracting the deps.json parsing functionality to a separate DLL to share with other hosting scenarios.

[AaronRobinsonMSFT]

form the component

"from"

[AaronRobinsonMSFT]

I like these strategy options - smells like an enum instead of a bool to me. Could also be a LoadIsolationPolicy struct.

[swaroop-sridhar]

Are you thinking of different kinds of isolation settings? For example:

• Load everything in the new ALC
• Load everything in the parent/default ALC (in which case, there is no point creating a new ALC)
• Load in the new ALC only if the parent cannot load it
• Load in the new ALC only if the parent cannot load it without using roll-forward
• ...

I think it is best to enable and leave these isolation settings to the developer, rather than providing API support for all of them -- at least to start with.

[AaronRobinsonMSFT]

Well the strategies are implying settings, not me. My point here is that a bool by its very nature is a binary idea, but based on all these talks it isn't a binary idea rather it is a spectrum. This implies a bool is not the appropriate idiom to use for isolation because it isn't "yes or no", but rather "isolate like this ...". An enum is one option and a "policy" type struct that indicate how to isolate is another. We are getting into the very concern you and I spoke about earlier this week which is why I am advocating for a very simply scenario that covers a majority of cases and then if a scenario exists that is distinct from the default the developer or consumer of this API will have to answer a series of questions and make real decisions.

rather than providing API support for all of them

That is a very nuanced statement. Yes and no. Yes, we shouldn't expose a plethora of options that aren't requested, but no we need to think about an API that can grow and adapt. enums and policy structs are the appropriate mechanisms for a malleable API surface, bools are not.

[vitek-karas]

Just curious which bool are you referring to? The public API as proposed above doesn't provide any way to customize the policy. We've been discussing this quite a bit and the current thinking has somewhat moved, but I'm still trying to prototype some of these ideas before writing it all down. The gist of it is:

• Provide some kind of building blocks, so that devs can compose the policy they want
• If we find a sensible default, implement that as a easy-to-use helper (or possibly the entry point into this problem space) - but this has not been agreed on yet.

[vitek-karas]

I've started the official review of this design here: dotnet/designs#47
I'm happy to continue some discussions here if you want to.

[vitek-karas]

This has been superceded by the AssemblyDependencyResolver work for the most part.