Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

Avoid a first-chance exception in ConflictResolver.ResolveConflict #326

Closed
KirillOsenkov opened this issue May 1, 2017 · 6 comments
Closed
Assignees

Comments

@KirillOsenkov
Copy link
Member

I hit a first-chance BadImageFormatException a lot with this stack:

 	mscorlib.dll!System.Reflection.AssemblyName.GetAssemblyName Line 213	C#
 	Microsoft.Packaging.Tools.Tasks.dll!Microsoft.DotNet.Build.Tasks.FileUtilities.GetAssemblyVersion	Unknown
 	Microsoft.Packaging.Tools.Tasks.dll!Microsoft.DotNet.Build.Tasks.FileUtilities.TryGetAssemblyVersion	Unknown
 	Microsoft.Packaging.Tools.Tasks.dll!Microsoft.DotNet.Build.Tasks.ConflictItem.AssemblyVersion.get	Unknown
 	Microsoft.Packaging.Tools.Tasks.dll!Microsoft.DotNet.Build.Tasks.ConflictResolver.ResolveConflict	Unknown
>	Microsoft.Packaging.Tools.Tasks.dll!Microsoft.DotNet.Build.Tasks.ConflictResolver.ResolveConflicts	Unknown
 	Microsoft.Packaging.Tools.Tasks.dll!Microsoft.DotNet.Build.Tasks.HandlePackageFileConflicts.Execute	Unknown

This is because here we try to get the assembly version of any .dll even if it's not a managed assembly:
https:/dotnet/standard/blob/master/Microsoft.Packaging.Tools/tasks/ConflictItem.cs#L56

I recommend using this simple code to first check whether a binary even represents a managed assembly:
https:/KirillOsenkov/MetadataTools/blob/master/PEFile/IsManagedAssembly.cs

Only attempt to retrieve the assembly name after we've checked that the binary is indeed an assembly.

@ericstj
Copy link
Member

ericstj commented May 15, 2017

/cc @dsplaisted this code lives in the SDK as well.
We do have a better implementation on core MSBuild: https:/dotnet/standard/blob/master/Microsoft.Packaging.Tools/tasks/FileUtilities.netstandard.cs#L23

On desktop though we just call GetAssemblyName. I suppose we could use the core implementation everywhere and it would avoid the exception.

@KirillOsenkov
Copy link
Member Author

Yes, that method doesn't throw on native binaries. Please use it :)

@ericstj
Copy link
Member

ericstj commented May 15, 2017

Ported to the SDK: dotnet/sdk#1215

@ericstj
Copy link
Member

ericstj commented May 17, 2017

Curious what file you're hitting this on. We'd only be going down this path if you had a native file conflict in your package closure, which shouldn't be the case for most projects if they are up-to-date.

@KirillOsenkov
Copy link
Member Author

Unfortunately I have no recollection of what I was doing and under what circumstances I've hit this. Sorry :)

@ericstj
Copy link
Member

ericstj commented May 18, 2017

We've decided that the tradeoffs for avoiding this first chance exception aren't worth it. I've opened https:/dotnet/corefx/issues/19928 to track adding an API that can be used to do this better in the future.

@ericstj ericstj closed this as completed May 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants