TypeLoadException when trying to attach to event of hosting application

[maettu-this]

A hosted script 'A' defines a script specific class TypeX. That TypeX accesses an event of the hosting application. The script can be started, the event can be handled. So far, all fine.

Another hosted script 'B' defines another script specific class TypeY. That TypeY also accesses the event of the hosting application. When trying the invoked that script, a TypeLoadException is thrown:

System.TypeLoadException: Der Typ "Test.ScriptB.TypeY" in der Assembly "Script, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null" konnte nicht geladen werden.
Stack:
bei System.Reflection.Assembly._GetType(String name, Boolean throwOnError, Boolean ignoreCase)
bei System.Reflection.MemberInfoSerializationHolder..ctor(SerializationInfo info, StreamingContext context) bei CSScriptLibrary.AsmBrowser.Invoke(Object obj, String methodName, Object[] list)
bei CSScriptLibrary.AsmRemoteBrowser.Invoke(String methodName, Object[] list)
Exception rethrown at [0]:
bei CSScriptLibrary.IAsmBrowser.Invoke(String methodName, Object[] list)
bei CSScriptLibrary.AsmHelper.Invoke(String methodName, Object[] list)
bei .InvokeAssembly(String assemblyFilePath, AsmHelper helper, String scriptMainMethod, Boolean scriptMainHasArgs)

Interestingly, this only happens if both scripts are compiled into an assembly with the same name, in this case Script.dll. If they are built into ScriptA.dll and ScriptB.dll, all works fine.

Additional findings:

• It doesn't matter whether type has same or different name.
• It doesn't matter whether type is MarshalByRefObject or not.
• It doesn't matter whether CSScript caching is enabled or not.
• It doesn't matter whether CSScript.ShareHostRefAssemblies is true or false.
• It does matter that script defined type is located in an assembly that has the same name as an assembly that got loaded before (e.g. Script.dll).
• It does matter that script defined type accesses the hosting application.

Take your time in evaluating this issue. I can live with it for the moment, as it is a more or less rare use case of my hosting application.

[maettu-this]

Here it is:
CSScriptHostingTest.zip

It's a test solution that condenses the relevant items of my hosting application. The solution consists of...
...your CSScriptLibrary (slightly modified to fit .NET 3.5),...
...a minimal ScriptHost project,...
...and test Scripts the reproduce the issue.

To reproduce:

1. Run ScriptHost in Debug
2. [Select] \Scripts\Issue_109\SameNamedScriptA\Script.cs
3. [Build]
4. [Run] => script outputs:
   "Script 1: This is ScriptA"
   "Script 1: Completed with result 0"
5. [Select] \Scripts\Issue_109\SameNamedScriptB\Script.cs
6. [Build]
7. [Run] => crash in AsmBrowser.Invoke()

Note that running the scripts in \Scripts\Issue_109\DifferentlyNamedScriptsAB works fine.

PS1:
Note the using (var resolveHelper = new AssemblyResolveHelper(...) at line 67 of Main.cs. It is required because CSScript.OnAssemblyResolve() is limited to the host domain, since options are initialized with CSScript.GlobalSettings, thus options.searchDirs only contains the assemblies of the current = host domain. But when the script in question tries to attach to a host's event handler, the host domain must be able to load the script type. Thus, the additional AssemblyResolveHelper resolves the missing types from the script domain into the host domain.

PS2 for FastInvoker:
GenerateMethodInvoker() (AsmHelper.cs line 1816) invoder => invoker
Invoke() (AsmHelper.cs line 1740) paramters => parameters

[oleg-shilo]

What is wrong with it?

In your scenario, you have two major players a host and a script. You want a script to be notified about some host events via a generic event handling mechanism. The problem is that this mechanism is not fit for the task. And I am not even talking about the notorious mem-leaking associated with the default .NET events implementation, which is based on strong (instead of weak) references. I am talking about the assembly hosting aspect of it.

BTW the problem is a generic .NET assembly hosting problem and it has nothing to do with scripting as such.

At startup, the host is instantiated in the AppDomain.Current (obviously). Then it compiles the script into an assembly and instantiates the script object into the remote AppDomain so it can be unloaded when no longer needed. So far so good. Both host and script assemblies belong to the different app domains and there is no collision between each other. But when the script object internally subscribes to the host event it's not only passing its reference to the host domain but also unvolunteerely triggers loading the script assembly into the host AppDomain. This can be easily verified with a simple probing code:

static string[] loaded_scripts()
{
    return AppDomain.CurrentDomain
                    .GetAssemblies()
                    .Where(x => x.FullName.StartsWith("Script")
                    .Select(x => x.Location)
                    .ToArray();
}

Place the checking in your button_Run_Click and you will see that after the scriptAsmHelper.Invoke call your "Script, 0.0.0.0" assembly appears to be loaded in the host AppDomain. And as you probably know it stays there forever.

Then you build and load the second script with the same assembly name "Script, 0.0.0.0". All good. But when you try to call scriptAsmHelper.Invoke, CLR automatically checks if the "Script, 0.0.0.0" assembly is already loaded and tries to use it. It finds it... but it is the first script assembly that has been accidentally loaded. From now everything is a mess.

How to fix it?

You need to stop your script assembly from being loaded in the host AppDomain. Get rid of event handling. You cannot use it. You cannot pass event handling delegates between AppDomains neither. They are not serializable.

You can only rely on interfaces.

This is your script code:
Note, you cannot use FormClosingEventArgs as it is not serializable.

using System;
using System.Windows.Forms;

using ScriptHost;

namespace Test.Issue_109.SameNamedScriptA
{
    public static class Script
    {
        public static int Main(ScriptHost.IScriptHost host)
        {
            host.WriteLine("This is ScriptA");

            var obj = new TypeX(host);
            obj.AttachEventHandler();
            host.Close();
            obj.DetachEventHandler();

            return (0);
        }
    }

    public class TypeX : MarshalByRefObject, IScriptHostEventHandler
    {
        protected IScriptHost Host { get; set; }

        public TypeX(IScriptHost host)
        {
            Host = host;
        }

        public virtual void AttachEventHandler()
        {
            Host.AttachEventHandler(this);
            // Host.Closing += Host_Closing;
        }

        public virtual void DetachEventHandler()
        {
            Host.DetachEventHandler(this);
            // Host.Closing -= Host_Closing;
        }

        // public void Host_Closing(object sender, FormClosingEventArgs e)
        public void Host_Closing(object sender, CloseReason e)
        {
            Host.WriteLine(string.Format("Host is closing due to '{0}'...", e));
        }
    }
}

And this is your hosting solution:

namespace ScriptHost
{
    public interface IScriptHostEventHandler
    {
        void Host_Closing(object sender, CloseReason e);
    }

    public interface IScriptHost
    {
        event EventHandler<FormClosingEventArgs> Closing;

        void AttachEventHandler(object obj);

        void DetachEventHandler(object obj);

        void WriteLine(string format, params object[] args);

        void Close();
    }

    public class Host : System.MarshalByRefObject, IScriptHost
    {
        List<IScriptHostEventHandler> scripts = new List<IScriptHostEventHandler>();

        public event EventHandler<FormClosingEventArgs> Closing;

        public void WriteLine(string format, params object[] args)
        {
            Debug.WriteLine(format, args);
        }

        public void AttachEventHandler(object obj)
        {
            scripts.Add((IScriptHostEventHandler)obj);
        }

        public void DetachEventHandler(object obj)
        {
            scripts.Remove((IScriptHostEventHandler)obj);
        }

        public void Close()
        {
            scripts.ForEach(s => s.Host_Closing(this, CloseReason.FormOwnerClosing));
        }
    }
}
...
static void Issue109()
{
    var script_file_a = @"E:\...\Issue_109\SameNamedScriptA\Script.cs";
    var script_file_b = @"E:\...\Issue_109\SameNamedScriptB\Script.cs";

    var asm_file_a = CSScript.CompileFile(script_file_a, Path.ChangeExtension(script_file_a, ".dll"), true);
    var asm_file_b = CSScript.CompileFile(script_file_b, Path.ChangeExtension(script_file_b, ".dll"), true);

    IScriptHost host = new Host();

    var local_dir = Path.GetDirectoryName(host.GetType().Assembly.Location);

    int result;

    using (SimpleAsmProbing.For(local_dir))
    using (var helper = new AsmHelper(asm_file_a, null, true))
    {
        result = (int)helper.Invoke("Test.Issue_109.SameNamedScriptA.Script.Main", host);
    }

    var asms = loaded_scripts();

    using (SimpleAsmProbing.For(local_dir))
    using (var helper = new AsmHelper(asm_file_b, null, true))
    {
        result = (int)helper.Invoke("Test.Issue_109.SameNamedScriptB.Script.Main", host);
    }
}

[maettu-this]

Hmm...

I agree that a script assembly should not get loaded into the host domain, what is done in this reduced test application. However, in my full application, I use a third domain:

Host domain (stays clean) <> ScriptRunner domain (may get polluted) <> Script domain

Each time a script has finished, the ScriptRunner domain gets unloaded as well, thus the host domain stays clean even when consecutively running scripts.

Let me change "stop your script assembly being loaded in the host AppDomain" into "stop your script assembly being loaded in the ScriptRunner AppDomain". Then, I no longer agree, because the script assembly must be loadable into the ScriptRunner AppDomain for another reason as well:

My host application implements a catch-all-handler which catches any exception thrown by a script and outputs full diagnostics information. Without this host-side catch-all-handler, each script would have to do that, quite inconvenient. In my application, this catch-all-handler is implemented in the ScriptRunner. Now, if the script throws a script defined exception type, that type must be loaded into the ScriptRunner domain, otherwise the host-side catch-all-handler cannot retrieve the exception information.

The very same applies to the event handling mechanism. Whenever a script has finished, the ScriptRunner domain gets unloaded and the event handlers fully get reset.

I agree that it doesn't properly work with the FormClosingEventArgs shown in the reduced test application. But I use [Serializable] event args in my full application, and this actually works fine.
Passing args from host to script is easy ([Serializable] clone of the data), returning data from script to host is also possible (as shown in https://stackoverflow.com/questions/22766549/cross-appdomain-cancelable-event).

I do agree that .NET standard event handling has issues, but still, it's standard, so people are used to it. And with some precautions, it does work fine. I really don't like doing things non-standard if the standard way is good enough.

For future reference, I will improve the event handling of the reduced test application such it no longer uses the non-[Serializable] FormClosingEventArgs. Also, I will add a note regarding the intermediate third AppDomain.

Though technically possible, I have my doubts that replacing the standard event handling mechanism by a non-standard approach will be beneficial for my hosting application, as I will have to document and explain more. Let's first see what you think about the more detailed explanations above.

[oleg-shilo]

Now, if the script throws a script defined exception type, that type must be loaded into the ScriptRunner domain, otherwise the host-side catch-all-handler cannot retrieve the exception information.

That's perfectly OK to have such a requirement. Simply the sample I had to work with did not have any reason to have the script assembly loaded into the caller domain. In fact I even see no reason why your ScriptRunner and the script have to be in the separate domains, as long as your host domain is untouched.

If the answer is "because ScriptRunner hosts multiple scripts" then you have the same assembly probing problem that you sample demonstrated. Though I only assume your answer. I just want to state that having the script assembly loaded at the same time in multiple appdomains has its cost.

I do agree that .NET standard event handling has issues, but still, it's standard, so people are used to it.

Don't overestimate the level of "being standard" for even handling. It has been known for years as a major cause of headache for GUI scenarios. If I am not mistaken WPF has ditched it in favor of a WeakreferenceEvents (Routed events) Caliburn.Micro (wonderful framework) uses WeakReferenced interfaces instead of events.

The bottom line is that you have to have script assemblies loaded to the caller domain.

1. If these assemblies have the same name you do mess up CLR assembly probing.
2. Having different assembly names solves the problem though potentially puts a bit of pressure on your loading/hosting solution.
3. And while redesigning the hosting model (e.g. via interfaces) solves the problem in the most natural way as it is completely decouples the script and the script runner. THough according you it is significantly less dev-friendly for your users.

The choice is yours. :)

[maettu-this]

Hmm again...

Thanks for the hint on not necessarily requiring separate AppDomain for ScriptRunner and Script. No, the ScriptRunner doesn't host multiple scripts, it is unloaded and recreated for each script. To me, it just looked more natural and better separated having two domains instead of a single ScriptRunner+Script domain. Also, the ScriptRunner domain only needs to load the script assembly in two specific cases:
a) Script attaches to event.
b) Script throws a user defined exception.
In all other cases, the ScriptRunner domain stays unspoiled as well.

For the test application, the TypeLoadException can be explained by the fact that the script assembly gets loaded into the host assembly. For the fully application, I will have to dig again. I suspect that the intermediate AppDomain doesn't fully decouple host and script domain, thus the issue actually happens back in the host domain. But that's just a guess.

Just for curiosity, I quickly googled around for Caliburn.Micro, seems that not everybody got convinced by it: https://softwareengineering.stackexchange.com/questions/287322/how-to-choose-not-to-use-a-framework-caliburn-micro-etc-in-a-given-mvvm-appl. Regarding "my users", they typically don't do MVVM. If using a form, it's just a message box or something rather simple. For such cases, WinForms is way good enough.

I will reconsider the AppDomain separation (three -vs- two) for a future version of my application. Also, I will then further dig into the TypeLoadException to find the root cause to it. For the moment, I will focus on releasing the current version, accepting the issue as a limitation.

I have one related question, then I suggest to close this issue:

In AsmHelper, what is the reason behind for limiting it to methods (e.g. GetStaticMethod(), Invoke(method))? The same applies to FastInvoker. As I understand, the runtime would also support invoking properties/fields/... But I am sure you had a good reason for limiting to methods.

[oleg-shilo]

I suspect that the intermediate AppDomain doesn't fully decouple host and script domain, thus the issue actually happens back in the host domain.

This would be my guess too. That var asms = loaded_scripts(); technique would really help you to detect the problems.

In AsmHelper, what is the reason behind for limiting it to methods...

AsmHelper objective is to allow script instantiation and auto-unloading. The ability to invoke a method was just a cherry on top. I would not recommend using AsmHelper for an intense API calls (props, fields). It is ultimately based on Reflection and as such it is not the best fit for the task. Just imagine calling with AsmHelper a method that has ref and out arguments.

Instead I always recommend using AsmHelper to instantiate your script object that implements your interface. Get the transparent_proxy script object, typecast it into the interface and you now have a normal C# experience. Intellisense, type safety, static typing, convenience... And as a positive side effect... probably slightly better design.

[oleg-shilo]

Just coudn't resist to reply. :)

not everybody got convinced by it...

There is a good bit in that discussion that you shared:

"I have found both WPF and Caliburn.Micro to be quite a steep learning curve, coming from WinForms, however after some experience with both I have found them a pleasure to use as a pair."

The learning extra effort is caused by the new binding paradigm but not by the framework event handling alternative (WeakReference+interface), which was the reason why I brought Caliburn.Micro as an example. In fact, I haven't found any criticism of Caliburn.Micro "events" at all.

In that very thread they discuss MVVMLight as an alternative to Caliburn.Micro. And surprise surprise.... It also ignores events in favor of what they call "WeakAction". Thus that discussion only confirms my point.

---

And on a non-technical note.

There are two products that I am personally truly impressed with. Caliburn.Micro and ServiceStack. I mean truly. These two remarkable products do not stand out simply because they are practically flawless on technical level but because they truly understood the cause of the problem they are trying to solve. And they offered unorthodox direct solution to the problem.

Note, I have no affiliation to these products whatsoever. :)

[oleg-shilo]

I am closing the issue but not the conversation :)