Just out of curiosity. Where you plan to plug this into Roslyn scripting? Take a look here for some of the details around this
https://github.com/dotnet/roslyn/tree/main/src/Scripting/Core/Hosting/AssemblyLoader

If I remember correctly this is where it stopped for me since everything in this area is either sealed or internal :)

Just out of curiosity. Where you plan to plug this into Roslyn scripting?

The current implementation of Roslyn already covers this by using its ownAssemblyLoadContext inhttps://github.com/dotnet/roslyn/blob/96d6017c378ab311544c5521a3e5c6616fea48a5/src/Scripting/Core/Hosting/AssemblyLoader/CoreAssemblyLoaderImpl.cs#L45. This should provide a proper assembly isolation.

The second set of changes presented above allows to configureScriptRunner class with a customAssemblyLoadContext.

Since we want to useAssemblyLoadContext class,Dotnet.Script.Core project now additionally targetsnetcoreapp2.1 framework.

The API ofScriptRunner class remains fully compatible with the existing code at source and binary levels. This is achieved by addingScriptRunner.AssemblyLoadContext property which has a getter and initializer (as opposed to a setter). In this way,ScriptRunner class remains immutable (as by design) and its public API does not impose breaking changes upon its customers.

I've also used nullable notation for that addition as the folks who switched to nullable checks reap the immediate benefits by seeing a question mark hints in IntelliSense dropdown, meaning that corresponding value is optional.

WhenScriptRunner.AssemblyLoadContext property is not set, the behavior ofScriptRunner class remains absolutely intact comparing to the baseline version. This provides a behavioral compatibility for the users.

(To be continued)

Just out of curiosity. Where you plan to plug this into Roslyn scripting?

The current implementation of Roslyn already covers this by using its ownAssemblyLoadContext inhttps://github.com/dotnet/roslyn/blob/96d6017c378ab311544c5521a3e5c6616fea48a5/src/Scripting/Core/Hosting/AssemblyLoader/CoreAssemblyLoaderImpl.cs#L45. This should provide a proper assembly isolation.

Yes, I know about that one :) I am the one who wrote most of the assembly loading stuff :)

What I meant was how to plug the new customAssemblyLoadContext into play so that theInteractiveAssemblyLoader actually uses this newAssemblyLoadContext

how to plug the new customAssemblyLoadContext into play so that theInteractiveAssemblyLoader actually uses this new AssemblyLoadContext

AssemblyLoadContext implementations seem to support semi-automatic chaining. When a custom context cannot resolve the assembly, it is automatically delegated to its parent. The parent is eitherAssemblyLoadContext.Default orAssemblyLoadContext.CurrentContextualReflectionContext for .NET Core 3.0+ hosts.

We will plug usingAssemblyLoadContext.EnterContextualReflection method, more on that later.

Design doc:https://github.com/dotnet/coreclr/blob/master/Documentation/design-docs/AssemblyLoadContext.ContextualReflection.md

Specific sample:https://github.com/dotnet/coreclr/blob/master/Documentation/design-docs/AssemblyLoadContext.ContextualReflection.md#maintaining-and-restoring-original-behavior. Note that allAssembly.Load calls inSomeCallbackMethod will be implicitly routed toCurrentContextualReflectionContext.

how to plug the new customAssemblyLoadContext into play so that theInteractiveAssemblyLoader actually uses this new AssemblyLoadContext

AssemblyLoadContext implementations support semi-automatic chaining. When a custom context cannot resolve the assembly, it is automatically delegated to its parent. The parent is eitherAssemblyLoadContext.Default orAssemblyLoadContext.CurrentContextualReflectionContext for .NET Core 3.0+ hosts.

We will plug usingAssemblyLoadContext.EnterContextualReflection method, more on that later.

Ah, I did not know about that one😊 Never a day without learning something new 😊

With the regards to the dependency on Gapotchenko.FX.Reflection.Loader I'd rather see that we inlined the needed bits if any.
Although this assembly is not used by OmniSharp, we would like to keep dependencies down to an absolute minimum.

The third set of changes presented above completes the local support ofAssemblyLoadContext inScriptRunner class,

As we know, the script is compiled to a temp folder and its dependencies are laying around at the same folder. The default logic embodied intoAssemblyLoadContext does not handle that scenario, so there is a need for a custom assembly loader that would handle that.

In order to achieve that, theAutoAssemblyLoader class is installed for the given assembly load context. It provides a scoped resolution for the just loaded assembly by handling assembly version selection, redirects, and unification of the dependencies. It comes fromGapotchenko.FX.Reflection.Loader module and has a pretty substantial implementation.

Other than providing a functional assembly loading, we should propagate ourAssemblyLoadContext instance downstream to the script we run. This is achieved by usingAssemblyLoadContext.EnterContextualReflection method for .NET Core 3.0+ targets.

(To be continued)

The last set of changes presented above glues and propagates a customAssemblyLoadContext through the higher level API. The compatibility at source and binary levels is preserved.

Here is an example of how it can be used:

class IsolatedLoadContext : AssemblyLoadContext{    protected override Assembly Load(AssemblyName assemblyName) => null;}Task<int> ExecuteScript(string filePath, IReadOnlyList<string> arguments){    var command = new ExecuteScriptCommand(ScriptConsole.Default, _LogFactory);    var scriptFile = new ScriptFile(filePath);    var commandOptions = new ExecuteScriptCommandOptions(scriptFile, arguments.ToArray(), OptimizationLevel.Release, null, false, false)    {        AssemblyLoadContext = new IsolatedLoadContext()    };    return command.Run<int, CommandLineScriptGlobals>(commandOptions);}

The sample demonstrates running the script in an isolated assembly load context. When script is being run in this way, its assemblies do not clash with the host assemblies.

If we do not specify a customAssemblyLoadContext then the behavior is the same as in baseline. So using a custom assembly load context is opt-in.

The commits above finish the core implementation of the feature.

With the regards to the dependency on Gapotchenko.FX.Reflection.Loader I'd rather see that we inlined the needed bits if any.

This module is used to the fullest extent here, so we would need almost all of the bits :)

I would vote against inlining of that particular one. If you dive deeper into the details, there are quite a few intricate aspects that should be handled in a particular and not always straightforward way.

This module encompasses maybe nearly 15 years of our work with .NET and custom assembly loading, but still, there are the days when we say "Aha!", learn something new, and stream those improvements into Gapotchenko.FX.Reflection.Loader.

I would really prefer if the maximum amount of living beings would benefit from that expertise, so really want to leave it upstreamed, not inlined. Reinventing the wheel is not energy-efficient and at some point of life you just want to rely on quality (which is hard to find, BTW).

However, a final decision is always yours and I would follow it.

This module encompasses maybe nearly 15 years of our work with .NET and custom assembly loading, but still, there are the days when we say "Aha!", learn something new, and stream those improvements into Gapotchenko.FX.Reflection.Loader.

We might be able to live with that although it is not my decision alone,@filipw also need to give his stamp of approval 👍

Just a couple of other things to be aware of

• Debugging - We need to verify that debugging scripts still work in VS Code.
• Add a test that verifies that this custom assembly loading actually solves the problem with a version conflict between the host and the script. We currently referenceNewtonsoft.Json 12.0.3. Maybe a test script which references an older version and verify that the version loaded is actually the one referenced in the script

#!/usr/bin/env dotnet-script#r "nuget:Newtonsoft.Json, 10.0.1"using System.Reflection;using Newtonsoft.Json;var versionAttribute = typeof(JsonConvert).Assembly.GetCustomAttributes<AssemblyInformationalVersionAttribute>().Single();Console.WriteLine(versionAttribute.InformationalVersion);

This currently spits out12.0.3+7c3d7f8da7e35dde8fa74188b0decff70f8f10e3 while it should have been10.0.1xxxxxx if the script is running in true isolation

The changes so far only addressed the API side of the feature, sodotnet-script is unable to isolate assemblies yet as it does not specify a custom assembly load context.

The next commits will gradually address those scenarios.

With the commit above, isolation is turned on for script file execution viadotnet-script command-line utility.

It has an interesting behavior. The script that references anewer Newtonsoft.Json is isolated correctly:

#r "nuget:Newtonsoft.Json, 13.0.1"using System.Reflection;using Newtonsoft.Json;var versionAttribute = typeof(JsonConvert).Assembly.GetCustomAttributes<AssemblyInformationalVersionAttribute>().Single();Console.WriteLine(versionAttribute.InformationalVersion);

It produces the expected output13.0.1+ae9fe44e1323e91bcbd185ca1a14099fba7c021f.

Without the isolation in place, the script just errors out withCould not load file or assembly 'Newtonsoft.Json, Version=13.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed'.

But, the script referencing anolder Newtonsoft.Json version10.0.1 with or without isolation in place gets satisfied with an already loaded Newtonsoft.Json 12.0.3 and produces12.0.3+7c3d7f8da7e35dde8fa74188b0decff70f8f10e3. It does not lead to the defects as Newtonsoft.Json is mostly binary compatible between the versions.

Should we really aim for the full assembly isolation? The existing behavior assuming assembly backward compatibility and partial isolation looks pretty sane.

IMHO I think for this to have real value, we should try to reach for the stars and provide 100% isolation. Otherwise we are possibly left with the same or additional edge cases related to inference from the host.

I think for this to have real value, we should try to reach for the stars and provide 100% isolation.

Agree. Meanwhile I was able to achieve 100% isolation in a prototype implementation. Should I continue in this pull request or is it better to split in two? The first one - API support which is safe and already covers some use cases, the second pull request - full isolation, includingdotnet-script.

I think we should try to keep this in a single PR. The changes so far is not too comprehensive. It would also be easier to review these changes as a whole. Great work btw. Really appreciate your efforts on this 👍👌

The current implementation reaches the full assembly isolation at API anddotnet-script levels.

There is a remaining thing: the full isolation of interactive runner. Currently it runs with a partial isolation which is pretty good.

I've tried to make it fully isolated and was blocked by this line in Roslyn:https://github.com/dotnet/roslyn/blob/2f6768efa3f5575bae411524bb11364b2208c2c4/src/Scripting/Core/Hosting/AssemblyLoader/CoreAssemblyLoaderImpl.cs#L35

Unfortunately an explicitAssemblyLoadContext.LoadAssemblyFromPath call is not routed to the context returned byAssemblyLoadContext.CurrentContextualReflectionContext. This means that we cannot intercept it without modifying Roslyn. So for now, only theScriptRunner class can be fully isolated.InteractiveRunner is at max partially isolated due to the way Roslyn works.

The pull requested is finished and ready for review/merge.

Impressive work indeed 👍. Currently in beer-mode, but I will have a look at this tomorrow 👍😊. My greatest concern here is the debugger. Have you tried debugging in VS Code using 100% isolation?

I testet this just briefly today and it looks very promising indeed. Also ran a quick test to ensure debugging still works. This probably means that we can get rid of a couple of hacks likehttps://github.com/filipw/dotnet-script/blob/2c21516f1f42ab16712af32b365a4e1966c29ca5/src/Dotnet.Script.Core/Dotnet.Script.Core.csproj#L32

Great suggestions regarding issues#166 and#268. I've removed the corresponding workarounds as they are no longer needed.

This is awesome 👍 Just one more thing

• <PackageReference Include="Gapotchenko.FX.Reflection.Loader" Version="2021.2.10-beta" /> Do you know if a stable version of this is available? We'd rather not have beta packages being referenced from a stable package 😀

We'd rather not have beta packages being referenced from a stable package

Good point. Fixed.

The following example documents the available levels of assembly isolation:

Task<int>ExecuteScript(stringfilePath,IReadOnlyList<string>arguments){varcommand=newExecuteScriptCommand(ScriptConsole.Default,_LogFactory);varscriptFile=newScriptFile(filePath);varcommandOptions=newExecuteScriptCommandOptions(scriptFile,arguments.ToArray(),OptimizationLevel.Release,null,false,false){AssemblyLoadContext=CreateAssemblyLoadContext()};returncommand.Run<int,CommandLineScriptGlobals>(commandOptions);}

1. No assembly isolation:

staticAssemblyLoadContext?CreateAssemblyLoadContext()=>null;

2. Partial assembly isolation:

sealedclassIsolatedLoadContext:AssemblyLoadContext{protectedoverrideAssemblyLoad(AssemblyNameassemblyName)=>null;}staticAssemblyLoadContext?CreateAssemblyLoadContext()=>newIsolatedLoadContext();

3. Full assembly isolation:

staticAssemblyLoadContext?CreateAssemblyLoadContext()=>newScriptAssemblyLoadContext();

ScriptAssemblyLoadContext class is provided byDotnet.Script.Core module and represents a fully isolated assembly load context. If so desired, the end user of the API can inherit that class and build on top of it.

4. Custom assembly isolation:

staticAssemblyLoadContext?CreateAssemblyLoadContext()=> ...;// custom implementation of assembly load context

@hrumhurum We just merged#633 which means that this PR needs to be updated to drop thenetcoreapp2.1 target framework 👍

The support for .NET Core 2.1 has been dropped.