Nerdbank.gitversioning: MarshalDirectiveException on unices with 2.2.3

[rainersigwald]

When I tried updating from 2.1.23 to 2.2.3 locally, I started seeing this failure repeated many times.

This does _not_ reproduce on the same machine with a tiny repro project, only in the MSBuild repo in our projects. I've reproduced on Ubuntu 18.04 (with SDK 2.1.300 and 2.1.401) and macOS 10.13 (just SDK 2.1.300), but it seems to work fine on .NET Core on Windows.

At the moment I suspect a CoreCLR bug rather than a problem with GitVersioning or LibGit2Sharp, but I wanted to file this for awareness in case it's more widespread.

10:41:29.576  1:19>Target "GetBuildVersion: (TargetId:201)" in file "/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/Nerdbank.GitVersioning.targets" from project "/home/raines/src/msbuild/src/Framework/Microsoft.Build.Framework.csproj" (target "GenerateAssemblyVersionInfo" depends on it):
                   Using "Nerdbank.GitVersioning.Tasks.GetBuildVersion" task from assembly "/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/MSBuildCore/Nerdbank.GitVersioning.Tasks.dll".
                   Task "Nerdbank.GitVersioning.Tasks.GetBuildVersion" (TaskId:117)
                     Task Parameter:TargetsPath=/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/ (TaskId:117)
10:41:29.604  1:19>/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/Nerdbank.GitVersioning.targets(63,5): error MSB4018: The "Nerdbank.GitVersioning.Tasks.GetBuildVersion" task failed unexpectedly. [/home/raines/src/msbuild/src/Framework/Microsoft.Build.Framework.csproj]
/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/Nerdbank.GitVersioning.targets(63,5): error MSB4018: System.Runtime.InteropServices.MarshalDirectiveException: StrictFilePathMarshaler must be used on a FilePath. [/home/raines/src/msbuild/src/Framework/Microsoft.Build.Framework.csproj]
/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/Nerdbank.GitVersioning.targets(63,5): error MSB4018:    at LibGit2Sharp.Core.StrictFilePathMarshaler.MarshalManagedToNative(Object managedObj) [/home/raines/src/msbuild/src/Framework/Microsoft.Build.Framework.csproj]
/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/Nerdbank.GitVersioning.targets(63,5): error MSB4018:    at System.StubHelpers.MngdRefCustomMarshaler.ConvertContentsToNative(IntPtr pMarshalState, Object& pManagedHome, IntPtr pNativeHome) [/home/raines/src/msbuild/src/Framework/Microsoft.Build.Framework.csproj]
/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/Nerdbank.GitVersioning.targets(63,5): error MSB4018:    at LibGit2Sharp.Core.NativeMethods.git_repository_open(git_repository*& repository, FilePath path) [/home/raines/src/msbuild/src/Framework/Microsoft.Build.Framework.csproj]
/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/Nerdbank.GitVersioning.targets(63,5): error MSB4018:    at LibGit2Sharp.Core.Proxy.git_repository_open(String path) [/home/raines/src/msbuild/src/Framework/Microsoft.Build.Framework.csproj]
/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/Nerdbank.GitVersioning.targets(63,5): error MSB4018:    at LibGit2Sharp.Repository..ctor(String path, RepositoryOptions options, RepositoryRequiredParameter requiredParameter) [/home/raines/src/msbuild/src/Framework/Microsoft.Build.Framework.csproj]
/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/Nerdbank.GitVersioning.targets(63,5): error MSB4018:    at Nerdbank.GitVersioning.GitExtensions.OpenGitRepo(String pathUnderGitRepo) [/home/raines/src/msbuild/src/Framework/Microsoft.Build.Framework.csproj]
/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/Nerdbank.GitVersioning.targets(63,5): error MSB4018:    at Nerdbank.GitVersioning.VersionOracle.Create(String projectDirectory, String gitRepoDirectory, ICloudBuild cloudBuild, Nullable`1 overrideBuildNumberOffset, String projectPathRelativeToGitRepoRoot) [/home/raines/src/msbuild/src/Framework/Microsoft.Build.Framework.csproj]
/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/Nerdbank.GitVersioning.targets(63,5): error MSB4018:    at Nerdbank.GitVersioning.Tasks.GetBuildVersion.ExecuteInner() [/home/raines/src/msbuild/src/Framework/Microsoft.Build.Framework.csproj]
                   Done executing task "Nerdbank.GitVersioning.Tasks.GetBuildVersion" -- FAILED. (TaskId:117)

[AArnott]

Thanks. Dupe or #214

[AArnott]

I've seen a repro on Windows, but like you, only on some projects.
I'll check out your idea that it's a .NET Core bug. I already investigated the libgit2sharp code and I don't see any way for this failure so a CLR bug isn't out of the question.

[rainersigwald]

I’ve got more info and will look again tomorrow but it looks like the FilePath type there is being loaded twice from the same file (but different clr assembly pointers).

[AArnott]

That might happen due to Assembly.LoadFrom, I think. I'll search nb.gv for use of that.

[AArnott]

No Assembly.Load of any variety in this repo. Hmmm...

[AArnott]

It repros for me on windows and linux, but so far I think always using dotnet build. msbuild.exe does not repro on Windows.

[AArnott]

I don't see the double assembly load. When I have the mixed mode debugger I see two, but one is just the native image representation.
I do see that the type is indeed correct, in name at least. So given the assembly is loaded once, and yet the type cast fails, it makes me question CoreCLR more. But there are load contexts in Core CLR, and an assembly can be loaded in multiple of them with different type identities as a result. I don't know if that shows up in the debugger as multiple assembly loads.
If libgit2sharp was somehow loaded in multiple load contexts, that might explain it. I'll research that tomorrow.

[rainersigwald]

I've been using this as a repro case: https://github.com/Microsoft/msbuild/blob/7125b9d66637d2babc88087b28cd4272133bd793/src/Framework/Microsoft.Build.Framework.csproj

I added some tracing in LibGit2Sharp at the point of failure:

diff --git a/LibGit2Sharp/Core/FilePathMarshaler.cs b/LibGit2Sharp/Core/FilePathMarshaler.cs
index 209254ac..5766e15c 100644
--- a/LibGit2Sharp/Core/FilePathMarshaler.cs
+++ b/LibGit2Sharp/Core/FilePathMarshaler.cs
@@ -66,6 +66,8 @@ public override IntPtr MarshalManagedToNative(Object managedObj)

             var filePath = managedObj as FilePath;

+            Console.WriteLine($"In {nameof(MarshalManagedToNative)}, managedObj: {managedObj} is a {managedObj.GetType()} from {managedObj.GetType().Assembly.FullName} ({managedObj.GetType().Assembly.Location}) and as a {typeof(FilePath).FullName} from {typeof(FilePath).Assembly.FullName} ({typeof(FilePath).Assembly.Location}) it's {filePath ?? "(null)"}. Type equality: {managedObj.GetType().Equals(typeof(FilePath))}");
+
             if (null == filePath)
             {
                 throw new MarshalDirectiveException(string.Format(CultureInfo.InvariantCulture,

In the repro, the task is run twice, once for each TargetFramework. The first one passes and the second fails, with

In MarshalManagedToNative, managedObj: /home/raines/src/msbuild/.git is a LibGit2Sharp.Core.FilePath from LibGit2Sharp, Version=0.25.0.0, Culture=neutral, PublicKeyToken=7cbde695407f0333 (/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/MSBuildCore/LibGit2Sharp.dll) and as a LibGit2Sharp.Core.FilePath from LibGit2Sharp, Version=0.25.0.0, Culture=neutral, PublicKeyToken=7cbde695407f0333 (/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/MSBuildCore/LibGit2Sharp.dll) it's /home/raines/src/msbuild/.git. Type equality: True
In MarshalManagedToNative, managedObj: /home/raines/src/msbuild/.git is a LibGit2Sharp.Core.FilePath from LibGit2Sharp, Version=0.25.0.0, Culture=neutral, PublicKeyToken=7cbde695407f0333 (/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/MSBuildCore/LibGit2Sharp.dll) and as a LibGit2Sharp.Core.FilePath from LibGit2Sharp, Version=0.25.0.0, Culture=neutral, PublicKeyToken=7cbde695407f0333 (/home/raines/.nuget/packages/nerdbank.gitversioning/2.2.3/build/MSBuildCore/LibGit2Sharp.dll) it's (null). Type equality: False

I set a breakpoint in the debugger and walked up through some of the structures at both calls. The calls have different objects (of course), but interestingly they had different class pointers as well . . . and chasing that up to the assembly level, they have different assemblies. Debugger transcript (rev 1 is first call, rev 2 is second): https://gist.github.com/rainersigwald/85746e07a1cda98a1458544ec6d9111f/revisions

The assemblies have different ClassLoader pointers, but that's a new concept for me and doesn't seem to be managed, so I'm not sure how to trace that divergence. AssemblyLoadContext would make sense (to my limited understanding).

[AArnott]

The repro for Windows and Ubuntu 18 is:

git clone https://github.com/AArnott/Nerdbank.Streams.git
cd Nerdbank.Streams/src/Nerdbank.Streams.Tests
git checkout 0b9733fd10615fafc2cfb7a51e9aedf5c5990529
dotnet build -f netcoreapp2.1

[AArnott]

I've sent an email to the NetFX folks asking for ideas.

[luqunl]

The assemblies have different ClassLoader pointers, but that's a new concept for me and doesn't seem to be managed, so I'm not sure how to trace that divergence. AssemblyLoadContext would make sense (to my limited understanding).

It is likely caused by multiple AssemblyLoadContext loads two copies LibGit2Sharp.dll.

[luqunl]

if it is easy, Could you try to "return Default.LoadFromAssemblyPath(assemblyPath);"
https://github.com/AArnott/Nerdbank.GitVersioning/blob/98359c20d4a989a0c6e9ab1c29fec129fad6055d/src/MSBuildExtensionTask/ContextAwareTask.cs#L86

[AArnott]

Thanks for looking, @luqunl. I wondered about that possibility. Before the p/invoke, when I create the FilePath object, I checked AssemblyLoadContext.GetLoadContext(typeof(FilePath).Assembly) and stamped that with the debugger $1 marker. Then in the custom marshaler after the cast failed, I checked that same expression again, and it was $1. That suggests to me that the load context for the type is the same in both frames.

[AArnott]

if it is easy, Could you try to "return Default.LoadFromAssemblyPath(assemblyPath);"

Yes, I'll try that.

[AArnott]

@luqunl That led to other errors around Newtonsoft.Json, v11 not being found.

[AArnott]

hang on... let me double-check something

[AArnott]

ya, I'm afraid your proposed change causes us to fail to find the newtonsoft.json that we expect.

[bording]

@AArnott What if you scope https://github.com/AArnott/Nerdbank.GitVersioning/blob/98359c20d4a989a0c6e9ab1c29fec129fad6055d/src/MSBuildExtensionTask/ContextAwareTask.cs#L83-L87 so that it is only used for LibGit2Sharp, similar to how it's done in https://github.com/dotnet/sourcelink/blob/master/src/Microsoft.Build.Tasks.Git/GitLoaderContext.cs#L18-L22 ?

[AArnott]

That's just it. If I don't use it for newtonsoft.json, then the Default just gives me back newtonsoft.json v9 because that's what the runtime (or msbuild?) uses instead of the v11 that I need. So it can't be so scoped.
But I just stepped through the debugger and confirmed that libgit2sharp always takes the L86 path. So as long as it's consistent, why would there be a problem?

[bording]

I was just comparing your AssemblyLoadContext implementation to the other one I'm aware of and happened to notice that difference. Seemed to be worth a mention at least.

[AArnott]

It was worth it. After trying what you said and it failing, I tried the opposite:

                AssemblyLoadContext preferredContext = assemblyName.Name.Equals("libgit2sharp", StringComparison.OrdinalIgnoreCase) ? Default : this;
                string assemblyPath = Path.Combine(this.loaderTask.ManagedDllDirectory, assemblyName.Name) + ".dll";
                if (File.Exists(assemblyPath))
                {
                    return preferredContext.LoadFromAssemblyPath(assemblyPath);
                }

So for libgit2sharp, while we still use the path, we use the Default context for it (but this for everything else). That seems to have made it work. I'm still testing to be sure...

[bording]

Just thought of another question. Are the places you're seeing this repro also using Microsoft.SourceLink.GitHub? If so, could there be some conflict there?

[AArnott]

That's an interesting question. Not in my case at least. I don't think any other MSBuild tasks are loading libgit2sharp in the Default load context either. But if there were, I could see how this workaround would fix the issue.

[AArnott]

Anyway, thanks for the suggestion, @bording. It looks like I'll be able to publish this workaround very soon.