-
-
Notifications
You must be signed in to change notification settings - Fork 173
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
Comments
Thanks. Dupe or #214 |
I've seen a repro on Windows, but like you, only on some projects. |
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).
|
That might happen due to Assembly.LoadFrom, I think. I'll search nb.gv for use of that. |
No Assembly.Load of any variety in this repo. Hmmm... |
It repros for me on windows and linux, but so far I think always using |
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'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
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). |
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 |
I've sent an email to the NetFX folks asking for ideas. |
It is likely caused by multiple AssemblyLoadContext loads two copies LibGit2Sharp.dll. |
if it is easy, Could you try to "return Default.LoadFromAssemblyPath(assemblyPath);" |
Thanks for looking, @luqunl. I wondered about that possibility. Before the p/invoke, when I create the |
Yes, I'll try that. |
@luqunl That led to other errors around |
hang on... let me double-check something |
ya, I'm afraid your proposed change causes us to fail to find the newtonsoft.json that we expect. |
@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 ? |
That's just it. If I don't use it for newtonsoft.json, then the |
I was just comparing your |
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 |
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? |
That's an interesting question. Not in my case at least. I don't think any other MSBuild tasks are loading libgit2sharp in the |
Anyway, thanks for the suggestion, @bording. It looks like I'll be able to publish this workaround very soon. |
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>
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.
The text was updated successfully, but these errors were encountered: