Stack overflow resolving TypeReference

[r-bennett]

In certain scenarios where a type implements an interface in another assembly, attempting to resolve the TypeReference to the interface results in an infinite loop and a stack overflow.

This is an issue I encountered with a real application with fairly complex dependencies - I've been struggling to find a simple repro that copies the right dlls in, but it's possible to manually copy them to the bin folder to reproduce.

The issue occurs when the build copies the following dlls to the bin dir:

  C:\Program Files\dotnet\shared\Microsoft.NETCore.App\2.1.0\System.dll
  C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\MSBuild\Microsoft\Microsoft.NET.Build.Extensions\net461\lib\System.Runtime.dll
  C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\MSBuild\Microsoft\Microsoft.NET.Build.Extensions\net461\lib\netstandard.dll 

The latter two get copied in when you have a net framework assembly which references a net standard assembly (as per some of the details in https://github.com/dotnet/corefx/issues/26456).

You can see the issue by creating a net461 library with a single type implementing ISet<string> (e.g. https://github.com/r-bennett/MonoCecilRepro/blob/master/ClassLibrary1/Class1.cs), building the library, manually copying the above dlls to the bin folder, then attempting to load the type and resolve its interface, something like:

void Main()
{
	ResolveInterfaces(@"E:\workspace\ClassLibrary1\ClassLibrary1\bin\Debug\ClassLibrary1.dll");
}

static void ResolveInterfaces(string assemblyPath)
{
	var module = ModuleDefinition.ReadModule(assemblyPath, new ReaderParameters { AssemblyResolver = new PublishedAssemblyResolver(assemblyPath) });
	foreach (var type in module.Types)
	{
		foreach (var @interface in type.Interfaces)
		{
			@interface.Resolve();
		}			
	}
}

public class PublishedAssemblyResolver : DefaultAssemblyResolver
{
	public PublishedAssemblyResolver (string assemblyPath)
	{
		AddSearchDirectory(Path.GetDirectoryName(assemblyPath));
	}
}

Each of the 3 dlls mentioned above has only a single type (<Module>) listed on it when read with Mono Cecil, with all the expected types appearing only in exportedTypes. The issue seems to be related to net standard's assembly unification (https://github.com/dotnet/standard/blob/315834b5eaa426ed37f6a767f15cf1d5eb7b4a85/docs/planning/netstandard-2.0/README.md#assembly-unification), although I'm not quite sure what's going on here.

The infinite loop occurs with MetadataResolver.Resolve(TypeReference) - it tries to resolve the ISet interface first with netstandard.dll, then with System.dll, then with System.Runtime.dll, then looping back to netstandard.dll when it can't find it in the types of any of those modules.

I'm happy to put some time into resolving this, but I'm not sure what the correct resolution behaviour should be here.

[jbevain]

Thanks for reporting this!

[MarcoRossignoli]

Confirm that the issue is the same, when resolution of type reach netstandard.dll for types forwarded to different assembly between netcoreapp/netfx it's not possibile find the implementation, in @r-bennett case:

Search System.Collections.Generic.ISet1 in System` (resolved in local folder 461 ver)

// Token: 0x27000087 RID: 135
.class extern forwarder System.Collections.Generic.ISet`1
{
	.assembly extern System.Runtime
}

Try to find on System.Runtime.dll (resolved in local folder 461 ver)

// Token: 0x270000B9 RID: 185
.class extern forwarder System.Collections.Generic.ISet`1
{
	.assembly extern netstandard
}

Try to find on netstandard.dll(resolved from local folder ver 461)

// Token: 0x27000064 RID: 100
.class extern forwarder System.Collections.Generic.ISet`1
{
	.assembly extern System
}

But you're on netcoreapp where System.Collections.Generic.ISet1 ( netstandard.dll copied from C:\Program Files\dotnet\shared\Microsoft.NETCore.App\2.2.2\netstandard.dll ) is on

// Token: 0x27000064 RID: 100
.class extern forwarder System.Collections.Generic.ISet`1
{
	.assembly extern System.Runtime
}

And so on.
I think that one possible solution could be in case of netstandard.dll not load from local but from typeof(object).Assembly.Location
In your case

public class PublishedAssemblyResolver : DefaultAssemblyResolver
{
    public PublishedAssemblyResolver(string assemblyPath)
    {
        AddSearchDirectory(Path.GetDirectoryName(assemblyPath));
    }

    public override AssemblyDefinition Resolve(AssemblyNameReference name)
    {
        if (name.FullName == "netstandard, Version=2.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51")
        {
            return AssemblyDefinition.ReadAssembly(Path.Combine(Path.GetDirectoryName(typeof(object).Assembly.Location), Path.ChangeExtension(name.Name, ".dll")));
        }
        else
        {
            return base.Resolve(name);
        }
    }
}

or first load always from "shared" framework(gac or shared)

public class PublishedAssemblyResolver : DefaultAssemblyResolver
{
    public PublishedAssemblyResolver(string assemblyPath)
    {
        AddSearchDirectory(Path.GetDirectoryName(typeof(object).Assembly.Location));
        AddSearchDirectory(Path.GetDirectoryName(assemblyPath));
    }
}

[r-bennett]

@MarcoRossignoli cheers for the info - have been having a poke around with your reproducer and been finding the same things.

I tried a workaround similar to the first you suggested, although checking when the only Type on the loaded assembly was <Module> (as it affects more than just netstandard.dll) and loading the assembly from Path.GetDirectoryName(typeof(object).Assembly.Location instead. This fixed this reproduction case. There are assemblies in the the "shared" framework which have the same issue though (e.g. in your reproducer which I converted to 461/2.0, the problem dll is C:\Program Files\dotnet\shared\Microsoft.NETCore.App\2.0.7\System.dll), so it isn't a universal fix.

The most robust solution I've found is to recursively add the subfolders from the GAC (under %systemroot%/assembly/...) as search directories, then Path.GetDirectoryName(typeof(object).Assembly.Location, then my assembly's publish directory last of all - I haven't found a project that doesn't work for yet.

[MarcoRossignoli]

FYI I'm trying to understand if "netstandard aware" resolver works well https://github.com/tonerdo/coverlet/pull/370/files#diff-b37379a034734abdfbec0ec6585cdfb7R630

[miwarnec]

@jbevain we are using Cecil for Mirror, our Unity networking library based on their old UNET.
We experienced the same issue. Are there any plans to fix it, or any suggestions for us to avoid it?

Edit: sorry for quadruple post, Github was bugged.

[jbevain]

Thanks to everybody who's looking at this.

My understanding of the issue lies in the very first Resolve call here. In a .NET Core program, you're asking the DefaultAssemblyResolver to resolve the assembly System for a program compiled for the .NET Framework and not .NET Core.

The DefaultAssemblyResolver implementation as it is today, when running on .NET Core, is going to resolve the .NET Core assembly for System, and here begins the mixup. The resolver will pick first the netstandard and System.Runtime for the .NET Framework that you provide yourself and this will never resolve properly.

I'm not sure there's a good solution here. If you're going to write a .NET Core program that can work on .NET Framework assemblies, the DefaultAssemblyResolver is not a good default, and you'll need to provide your own which knows how to resolve .NET Framework references when working on a .NET Framework assembly.

[SimonCropp]

agree with @jbevain on

If you're going to write a .NET Core program that can work on .NET Framework assemblies, the DefaultAssemblyResolver is not a good default, and you'll need to provide your own which knows how to resolve

this was my finding when writing Fody. assembly resolution is very specific to the context cecil is being run from. DefaultAssemblyResolver does a pretty good job for many scenarios, but it is not possible for it to cover all context/use cases that cecil can be run from

[MarcoRossignoli]

Yep on coverlet custom resolver seems to work well.

[jbevain]

Closing this as per #573 (comment)

I'm not sure the default resolver could do a lot more for those mixed scenarios. Thanks!