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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MarshalDirectiveException on unices with 2.2.3 #215

Closed
rainersigwald opened this issue Aug 23, 2018 · 24 comments
Closed

MarshalDirectiveException on unices with 2.2.3 #215

rainersigwald opened this issue Aug 23, 2018 · 24 comments
Assignees
Labels

Comments

@rainersigwald
Copy link
Member

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
Copy link
Collaborator

AArnott commented Aug 24, 2018

Thanks. Dupe or #214

@AArnott AArnott closed this as completed Aug 24, 2018
@AArnott
Copy link
Collaborator

AArnott commented Aug 24, 2018

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
Copy link
Member Author

rainersigwald commented Aug 24, 2018 via email

@AArnott
Copy link
Collaborator

AArnott commented Aug 24, 2018

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

@AArnott
Copy link
Collaborator

AArnott commented Aug 24, 2018

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

@AArnott
Copy link
Collaborator

AArnott commented Aug 24, 2018

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
Copy link
Collaborator

AArnott commented Aug 24, 2018

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
Copy link
Member Author

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
Copy link
Collaborator

AArnott commented Aug 24, 2018

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
Copy link
Collaborator

AArnott commented Aug 24, 2018

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

@luqunl
Copy link

luqunl commented Aug 24, 2018

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
Copy link

luqunl commented Aug 24, 2018

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
Copy link
Collaborator

AArnott commented Aug 24, 2018

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
Copy link
Collaborator

AArnott commented Aug 24, 2018

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

Yes, I'll try that.

@AArnott
Copy link
Collaborator

AArnott commented Aug 24, 2018

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

@AArnott
Copy link
Collaborator

AArnott commented Aug 24, 2018

hang on... let me double-check something

@AArnott
Copy link
Collaborator

AArnott commented Aug 24, 2018

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

@AArnott
Copy link
Collaborator

AArnott commented Aug 25, 2018

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
Copy link

bording commented Aug 25, 2018

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
Copy link
Collaborator

AArnott commented Aug 25, 2018

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
Copy link

bording commented Aug 25, 2018

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
Copy link
Collaborator

AArnott commented Aug 25, 2018

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 added a commit that referenced this issue Aug 25, 2018
@AArnott
Copy link
Collaborator

AArnott commented Aug 25, 2018

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

AArnott added a commit that referenced this issue Aug 25, 2018
AArnott pushed a commit that referenced this issue Sep 14, 2023
Bumps [dotnet-coverage](https://github.com/microsoft/codecoverage) from 17.8.2 to 17.8.4.
- [Commits](https://github.com/microsoft/codecoverage/commits)

---
updated-dependencies:
- dependency-name: dotnet-coverage
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants