oleg-shilo / cs-script

C# scripting platform
http://www.cs-script.net
MIT License
1.62k stars 235 forks source link

Unload assembly crashes if GC happaned #301

Closed guffy1234 closed 1 year ago

guffy1234 commented 2 years ago

hi

Unload assembly crashes with System.ExecutionEngineException if garbage collection happened between script compilation and unloading. Simplest example:

var assembly = CSScript.Evaluator .With(eval => eval.IsAssemblyUnloadingEnabled = true) .CompileCode(@"public int Sum(int a, int b) { return a+b; }");

GC.Collect();

assembly.Unload();

oleg-shilo commented 2 years ago

An interesting problem.

Assembly.Unload is a very thin extension method that simply offers syntactic sugar around AssemblyLoadContext:

https://github.com/oleg-shilo/cs-script/blob/d71249d3feb69da9ba03d99f16e82726dad8c399/src/cscs/Utils/ReflectionExtensions.cs#L49-L65

Thus why calling AssemblyLoadContext.Unload causes an exception is really a mystery that is originated somewhere in the depth of System.Runtime.Loader.dll. And the problem is so severe that it s cracking the whole AppDomain. The exception cannot be even caught.

It's not the first problem Unload. Thus if the script object is declared as dynamic unloading is... silently not happening -> effectively a memory leak.

That's why I disabled unloading functionality by default but still left it in the product because I hope one day the dotnet team will actually resolve these issues.

guffy1234 commented 2 years ago

Yep. Runtime become so damaged so couldn't handle exception.

If you interesting in to separate the issue from your scripting engine codes and have a time to experiments - you may create a sample where you just load an assemply dll, then perform GC and then try to unload the context. if this sample will crash too - then any plugin engine is in risk

oleg-shilo commented 2 years ago

Already done. I mean I have already captured an example that shows the conditions when AssemblyLoadContext.Unload does not work.

But I agree it needs to be extended with an extra example to show this nasty problem too. Done too.

https://github.com/oleg-shilo/cs-script/blob/f1562e4724350e987b22f5d5ea787bfeb0851df3/src/CSScriptLib/src/Client.NET-Core/Program.cs#L112-L123

guffy1234 commented 2 years ago

гаразд, друже. дякую за гарний продукт. :clap: якщо треба допомога - пиши. :slightly_smiling_face: :blue_square: :yellow_square:

oleg-shilo commented 2 years ago

Слава Українi! Трумайтесь. Австралiя з вами

Jeroen-Smits commented 2 years ago

Hi,

I also ran into this issue, but because I needed the ability to at runtime update some scripts, I investigated the issue and I think I found a workaround.

image

I didn't have the fatal error when I resolved the AssemblyLoadContext before the GC.Collect. So now I resolve the context just after compiling the script and nolonger run into this issue. Unfortunately, you cannot use the Unload extension method.

Hope this will help.