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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make C# Loader production ready #123

Open
viferga opened this issue Apr 22, 2021 · 2 comments
Open

Make C# Loader production ready #123

viferga opened this issue Apr 22, 2021 · 2 comments
Labels
c/c++ Pull requests that update C/C++ code csharp Pull requests that update C# code enhancement New feature or request

Comments

@viferga
Copy link
Member

viferga commented Apr 22, 2021

馃殌 Feature

Right now the C# Loader does not follow the interface of MetaCall properly. This is very problematic because all scripts are loaded into the current AppDomain and this will allow collisions between dependencies. Apart from that, there is another problem related to the dependencies itself, they are usually not tracked properly and fail to load because the AppDomain cannot locate them. I have been playing around with them with a PoC of Blazor, and it shows an error related to strong naming (https://stackoverflow.com/questions/16674315/understanding-appdomains-and-strong-naming).

Here's an explanation of how to load assemblies with dependencies: https://stackoverflow.com/a/37060921
The repository of the PoC is here: https://github.com/mathume/ResolveDependencies
I have attached it as a zip too: ResolveDependencies-master.zip

The current implementation does not return a handle when loading, instead all functions are loaded into a singleton. As you can see, it returns boolean instead of a handle pointer.

  1. C# Side:
    public unsafe static bool LoadFilesW([MarshalAs(UnmanagedType.LPArray, SizeParamIndex = 1)]IntPtr[] source, long size)
  2. NetCore Interface:
    return this->core_load_from_files_w(source, size) > 0 ? true : false;
  3. Loader Interface:
    return (loader_handle)impl;

This breaks the design, it should return a structure in the 3) containing a pointer to the handle returned by 1), which should contain a pointer to the new AppDomain (one by handle) and a pointer to the list of loaded assemblies or similar, so later on the functions can be retrieved and they are scoped by handle, instead of by loader instance like now:

public static void GetFunctions(ref int count, IntPtr p)

The good part is that the function container already points to their own assembly so it will be simpler to migrate:

this.Assembly = info.Module.Assembly;

Apart from that, there's part of the C/C++ API using the old embedding API, now the current embedding API of .NET Core has been simplified. I am not sure but this may be related with another issue which is, even updating to NET Core to version 5, it shows the following:

CSLoader Initialization - Runtime Version: v4.0.30319 - Language Version: 7.3

This is problematic because it should be version 5.0.5 of the runtime and version 9.0 of the language.

The new hosting API documentation can be found here: https://docs.microsoft.com/en-us/dotnet/core/tutorials/netcore-hosting

This is the methodology that we are using now (for all versions, including 1, 2 and 5):

The preferred method of hosting the .NET Core runtime prior to .NET Core 3.0 is with the coreclrhost.h API. This API exposes functions for easily starting and stopping the runtime and invoking managed code (either by launching a managed exe or by calling static managed methods).

And this is the one that we may need to change.

The preferred method of hosting the .NET Core runtime in .NET Core 3.0 and above is with the nethost and hostfxr libraries' APIs. These entry points handle the complexity of finding and setting up the runtime for initialization and allow both launching a managed application and calling into a static managed method.

I am not sure if the error of Runtime Version: v4.0.30319 - Language Version: 7.3 is completely related to this, I don't know NET Core in depth and I haven't investigated enough yet, but I am providing information for the reader.

Implementing classes and objects can be also a plus, because now we do support them, and C# has a great API to introspect the elements so probably it's easy to extend. Here's where it loads the static methods from all functions:

foreach (var item in assembly.DefinedTypes.SelectMany(x => x.GetMethods()).Where(x => x.IsStatic))

Other extra features can be supporting F# and VB.NET, but those are not a priority. Doing the work already described will be enough to make this loader ready for production.

Is your feature request related to a problem?

Yes, this generates two problems:

  1. We cannot support real world projects with dependencies (specially compiled assemblies).
  2. We cannot support C# 9.0 which is required in order to support top level expressions, so we can use those as lambdas (or functions) instead of static methods in classes as we are doing now. This should be supported with the Class type with the new MetaObject Protocol API.

Describe the solution you'd like

I am open to the implementation, but I described in the first point what is the path that the implemenator must follow.

Describe alternatives you've considered

Maybe it is possible to workaround those problems without needing to reimplement parts of the Loader. I have tried without success so probably the best solution is this refactor.

Additional context

Please check the NET Core / C# documentation, it is excellent.

@viferga viferga added csharp Pull requests that update C# code enhancement New feature or request c/c++ Pull requests that update C/C++ code labels Apr 22, 2021
@viferga
Copy link
Member Author

viferga commented May 11, 2021

Also, some leaks obtained from address sanitizer:

output.txt

@viferga
Copy link
Member Author

viferga commented Jan 31, 2023

Additional problems:
#378
#377

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c/c++ Pull requests that update C/C++ code csharp Pull requests that update C# code enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant