Fix C# native library loading in .NET 5 single-file apps #24744

jtattermusch · 2020-11-13T16:06:50Z

jtattermusch · 2020-11-13T16:10:25Z

Once this is approved, I'll also create a backport PR.

jtattermusch · 2020-11-13T16:11:23Z

Adhoc distribtests: https://g3c.corp.google.com/results/invocations/a25cde5a-bf1c-49cc-923e-315e96987e17 (including the newly added single file publish distribtest).

jtattermusch · 2020-11-13T16:15:37Z

src/csharp/Grpc.Core/Internal/NativeExtension.cs

        {
            var assembly = typeof(NativeExtension).GetTypeInfo().Assembly;
 #if NETSTANDARD
            // Assembly.EscapedCodeBase does not exist under CoreCLR, but assemblies imported from a nuget package
            // don't seem to be shadowed by DNX-based projects at all.
-            return assembly.Location;
+            var assemblyLocation = assembly.Location;
+            if (assemblyLocation != null)


Actually, assembly.Location might be returning an empty string according to the docs:
https://docs.microsoft.com/en-us/dotnet/core/deploying/single-file#api-incompatibility

I'll check which one is correct and update the code if needed.

jtattermusch · 2020-11-13T16:22:40Z

Another adhoc distribtest run: https://g3c.corp.google.com/results/invocations/8872dc43-c7da-4533-b792-bc1c5eaff03b/targets

src/csharp/Grpc.Core/Internal/NativeExtension.cs

jtattermusch · 2020-11-13T16:49:48Z

Yet another adhoc distribtest: https://g3c.corp.google.com/results/invocations/700ebfdb-8e9a-43c8-9e43-2e58a4929037

jkotas · 2020-11-13T16:51:20Z

https://github.com/grpc/grpc/blob/master/src/csharp/Grpc.Core/Internal/UnmanagedLibrary.cs#L245 is another problem that needs to be fixed to make the single file scenario working. There is no libcoreclr.so on disk in the single file scenario.

jtattermusch · 2020-11-13T17:39:05Z

https://github.com/grpc/grpc/blob/master/src/csharp/Grpc.Core/Internal/UnmanagedLibrary.cs#L245 is another problem that needs to be fixed to make the single file scenario working. There is no libcoreclr.so on disk in the single file scenario.

Ok, thanks for the tip. Do you have ideas on how to fix this problem?

gitfool · 2020-11-13T17:52:47Z

@jtattermusch please add a distribtest for the IncludeNativeLibrariesForSelfExtract case. Hopefully @jkotas can advise on how to detect and/or handle that too?

jkotas · 2020-11-13T18:09:20Z

Do you have ideas on how to fix this problem?

The best way to fix this problem is to use Native.Load for .NET Core. This API deals with all probing paths, single file and platform differences, including IncludeNativeLibrariesForSelfExtract.

jtattermusch · 2020-11-16T07:31:36Z

Native.Load

Thanks for the tip! Unfortunately Native.Load is only available on .NET Core 3+ (and at this point the newest we target is netstandard2.0)

jtattermusch · 2020-11-16T11:43:11Z

Another distribtest: https://g3c.corp.google.com/results/invocations/757851c5-c719-4434-bad6-bac0e655990b/targets

jtattermusch · 2020-11-16T15:01:46Z

The interop test failure is #24764.

jtattermusch · 2020-11-16T15:10:12Z

As @jkotas has pointed out, I'm now getting the Unhandled exception. System.DllNotFoundException: Unable to load shared library 'libcoreclr.so' error.

https://source.cloud.google.com/results/invocations/9ccd7ae8-32c0-4ee9-b08a-e7848b2f84a4/targets/github%2Fgrpc/tests

grpc/src/csharp/Grpc.Core/Internal/UnmanagedLibrary.cs

Line 245 in 014a2a0

[DllImport("libcoreclr.so")]

jtattermusch · 2020-11-17T18:02:05Z

@yulin-liang I don't have a fully working fix at this point, so I think it's fine to cut the release branch without it. (If I come up with a fix, I can backport it)

jkotas · 2020-11-24T16:30:07Z

Unfortunately Native.Load is only available on .NET Core 3+ (and at this point the newest we target is netstandard2.0)

The fix for this should be to try to lookup these methods using reflection, and then use them instead of the hardcoded P/Invokes when these methods are available. Something like:

delegate bool TryLoadDelegate(string libraryName, System.Reflection.Assembly assembly, System.Runtime.InteropServices.DllImportSearchPath? searchPath, out IntPtr handle);
delegate bool TryGetExportDelegate(IntPtr handle, string name, out IntPtr address);

static TryLoadDelegate s_tryLoad;
static TryGetExportDelegate s_tryGetExport; 

...

Type nativeLibrary = Type.GetType("System.Runtime.InteropServices.NativeLibrary, System.Runtime.InteropServices, PublicKeyToken=b03f5f7f11d50a3a", throwOnError: false);
if (nativeLibrary == null)
    return;

MethodInfo tryLoad = nativeLibrary.GetMethod("TryLoad", BindingFlags.Static | BindingFlags.Public, null,
    new Type[] { typeof(string), typeof(Assembly), typeof(Nullable<DllImportSearchPath>), typeof(IntPtr).MakeByRefType() }, null);
s_tryLoad = (TryLoadDelegate)tryLoad?.CreateDelegate(typeof(TryLoadDelegate));

MethodInfo tryGetExport = nativeLibrary.GetMethod("TryLoad", BindingFlags.Static | BindingFlags.Public, null,
    new Type[] { typeof(IntPtr), typeof(string), typeof(IntPtr).MakeByRefType() }, null);
s_tryGetExport = (TryGetExportDelegate)tryGetExport?.CreateDelegate(typeof(TryGetExportDelegate));

jtattermusch · 2020-11-25T09:24:16Z

Alternative approach used by a library that faced a similar problem: gui-cs/Terminal.Gui@19955fa#diff-91d94d0e5b8dd01a182c51299d7e8093247fa636a9768574b7042bfd5df9a1deR261

jtattermusch · 2020-11-30T11:03:02Z

I refactored the native library loading logic to use the default DllImport loading mechanism when on .NET Core.
It leads to some code simplification and also seems to handle the case with of .NET 5 single file published app well.
PTAL.

The adhoc distribtest run is here: https://g3c.corp.google.com/results/invocations/4da3122d-8972-48ff-ac92-d01b22705b99/log

jtattermusch · 2020-11-30T13:26:35Z

Known failures: build_docker_go is also failing on master.

jtattermusch · 2020-11-30T15:01:25Z

Adhoc distribtests are passing:
https://g3c.corp.google.com/results/invocations/50ae873b-5510-4ae0-b735-0840c7e9dd6d/targets/grpc%2Fcore%2Fexperimental%2Fgrpc_distribtests_multiplatform/log (the php7 distribtest has actually failed but that's unrelated to this PR - everything C# related is green)

dotnet 5 distribtest is also green:
https://g3c.corp.google.com/results/invocations/4228feb1-e59b-4bb1-9933-982998e8d431/targets/github%2Fgrpc;config=default/tests;query=csharp

jtattermusch · 2020-11-30T15:03:51Z

@jkotas your opinions on the approach I ended up taking (= rely on DllImport default loading logic) are welcome.

@apolcyn PTAL.

jkotas · 2020-11-30T15:20:33Z

src/csharp/Grpc.Core/Internal/NativeExtension.cs

        {
            // TODO: allow customizing path to native extension (possibly through exposing a GrpcEnvironment property).
            // See https://github.com/grpc/grpc/pull/7303 for one option.
-            var assemblyDirectory = Path.GetDirectoryName(GetAssemblyPath());
+            var assemblyDirectory = GetAssemblyDirectory();


The changes in this method should not be needed with your new fix.

jkotas · 2020-11-30T15:23:23Z

src/csharp/Grpc.Core/Internal/NativeExtension.cs

+                // The classes with the list of DllImport'd methods are code generated,
+                // so having more than just one doesn't really bother us.
+
+                // on Windows, the DllImport("grpc_csharp_ext.x64") doesn't work for some reason,


This was bug dotnet/coreclr#17505 . It is fixed in .NET Core 3.1, but I assume you still want this to work on .NET Core 2.1.

jkotas · 2020-11-30T15:24:25Z

@jkotas your opinions on the approach I ended up taking

Looks fine to me.

gitfool · 2020-11-30T17:29:14Z

@jtattermusch does your fix handle the IncludeNativeLibrariesForSelfExtract case too?

jtattermusch · 2020-12-01T13:07:22Z

@jtattermusch does your fix handle the IncludeNativeLibrariesForSelfExtract case too?

I haven't tested that explicitly - would you mind trying it out and posting back the result?

The nightly build of the nugets should appear here soon:
https://packages.grpc.io/

Alternatively, here are nugets from my last manual build:
https://storage.cloud.google.com/grpc-testing-kokoro-prod/test_result_public/prod/grpc/core/master/linux/grpc_build_packages/14907/20201130-052042/github/grpc/artifacts/csharp_nugets_windows_dotnetcli.zip

I wanted to merge this PR ASAP because like this the fixes will still make it to Grpc.Core 2.34.0 release - we can address IncludeNativeLibrariesForSelfExtract as a follow up.

gitfool · 2020-12-07T20:57:47Z

@jtattermusch I did a test with Grpc.Core version 2.34.0 and it works for IncludeNativeLibrariesForSelfExtract too! 😀

jtattermusch added the priority/P1 label Nov 13, 2020

jtattermusch assigned apolcyn Nov 13, 2020

jtattermusch added lang/C# release notes: yes Indicates if PR needs to be in release notes labels Nov 13, 2020

jtattermusch requested a review from apolcyn November 13, 2020 16:07

jtattermusch added priority/P0/RELEASE BLOCKER and removed priority/P1 labels Nov 13, 2020

jtattermusch mentioned this pull request Nov 13, 2020

net5.0 'publish single file' makes LoadUnmanagedLibrary fail with ArgumentNullException #24266

Closed

jtattermusch commented Nov 13, 2020

View reviewed changes

jkotas reviewed Nov 13, 2020

View reviewed changes

src/csharp/Grpc.Core/Internal/NativeExtension.cs Outdated Show resolved Hide resolved

apolcyn approved these changes Nov 13, 2020

View reviewed changes

jtattermusch force-pushed the net5_singlefile_publish branch from dce96e9 to 4e8de8d Compare November 16, 2020 11:30

jtattermusch added priority/P1 and removed priority/P0/RELEASE BLOCKER labels Nov 17, 2020

add C# distribtest coverage for .NET5 single file publish

53390d9

jtattermusch added 3 commits November 30, 2020 10:12

fix native extension loading in .NET5 single-file deployments

1f9f3c7

Load native extension on .NET core via differentiated DllImports

7324c5c

simplify .NET framework loading logic

840590e

jtattermusch force-pushed the net5_singlefile_publish branch from 77212fd to 840590e Compare November 30, 2020 10:56

jkotas reviewed Nov 30, 2020

View reviewed changes

apolcyn approved these changes Nov 30, 2020

View reviewed changes

jtattermusch merged commit 0ebae4e into grpc:master Nov 30, 2020

This was referenced Nov 30, 2020

Fix C# native library loading in .NET 5 single-file apps (backport to v1.34.x) #24853

Merged

DRAFT: Get rid of C# UnmanagedLibrary logic entirely #24854

Closed

jtattermusch mentioned this pull request Dec 3, 2020

.NET 5.0 apps require libc-dev #24153

Closed

This was referenced Feb 17, 2021

gRPC call hangs on Linux x64 in Azure Functions host #25208

Closed

C#: Use explicit native extension loading whenever possible #25490

Merged

jtattermusch mentioned this pull request Feb 23, 2022

Error loading native library when running application under IIS #24495

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix C# native library loading in .NET 5 single-file apps #24744

Fix C# native library loading in .NET 5 single-file apps #24744

jtattermusch commented Nov 13, 2020

jtattermusch commented Nov 13, 2020

jtattermusch commented Nov 13, 2020

jtattermusch Nov 13, 2020

jtattermusch commented Nov 13, 2020

jtattermusch commented Nov 13, 2020

jkotas commented Nov 13, 2020

jtattermusch commented Nov 13, 2020

gitfool commented Nov 13, 2020

jkotas commented Nov 13, 2020 •

edited

Loading

jtattermusch commented Nov 16, 2020

jtattermusch commented Nov 16, 2020

jtattermusch commented Nov 16, 2020

jtattermusch commented Nov 16, 2020 •

edited

Loading

jtattermusch commented Nov 17, 2020

jkotas commented Nov 24, 2020

jtattermusch commented Nov 25, 2020

jtattermusch commented Nov 30, 2020

jtattermusch commented Nov 30, 2020

jtattermusch commented Nov 30, 2020

jtattermusch commented Nov 30, 2020

jkotas Nov 30, 2020

jkotas Nov 30, 2020

jkotas commented Nov 30, 2020

gitfool commented Nov 30, 2020

jtattermusch commented Dec 1, 2020

gitfool commented Dec 7, 2020

Fix C# native library loading in .NET 5 single-file apps #24744

Fix C# native library loading in .NET 5 single-file apps #24744

Conversation

jtattermusch commented Nov 13, 2020

jtattermusch commented Nov 13, 2020

jtattermusch commented Nov 13, 2020

jtattermusch Nov 13, 2020

Choose a reason for hiding this comment

jtattermusch commented Nov 13, 2020

jtattermusch commented Nov 13, 2020

jkotas commented Nov 13, 2020

jtattermusch commented Nov 13, 2020

gitfool commented Nov 13, 2020

jkotas commented Nov 13, 2020 • edited Loading

jtattermusch commented Nov 16, 2020

jtattermusch commented Nov 16, 2020

jtattermusch commented Nov 16, 2020

jtattermusch commented Nov 16, 2020 • edited Loading

jtattermusch commented Nov 17, 2020

jkotas commented Nov 24, 2020

jtattermusch commented Nov 25, 2020

jtattermusch commented Nov 30, 2020

jtattermusch commented Nov 30, 2020

jtattermusch commented Nov 30, 2020

jtattermusch commented Nov 30, 2020

jkotas Nov 30, 2020

Choose a reason for hiding this comment

jkotas Nov 30, 2020

Choose a reason for hiding this comment

jkotas commented Nov 30, 2020

gitfool commented Nov 30, 2020

jtattermusch commented Dec 1, 2020

gitfool commented Dec 7, 2020

jkotas commented Nov 13, 2020 •

edited

Loading

jtattermusch commented Nov 16, 2020 •

edited

Loading