Defend against cyclical Type Forwarders?

[KirillOsenkov]

This is not necessarily a bug in Cecil per se, but something to think about to protect consumers against cyclical type forwarders.

I'm filing this primarily so that there's a record of a lengthy investigation where the symptom was a StackOverflow in Cecil with this repeating stack:

Mono.Cecil.ExportedType.Resolve <--
Mono.Cecil.MetadataResolver.GetType
Mono.Cecil.MetadataResolver.Resolve
Mono.Cecil.ModuleDefinition.Resolve
Mono.Cecil.ExportedType.Resolve  <--
Mono.Cecil.MetadataResolver.GetType
Mono.Cecil.MetadataResolver.Resolve
Mono.Cecil.ModuleDefinition.Resolve
Mono.Cecil.TypeReference.Resolve
Mono.Cecil.TypeReference.ResolveDefinition
Mono.Cecil.MemberReference.Resolve

I'm attaching two .dlls where the type System.Lazy``2 is type forwarded to the other .dll.

CecilRepro.zip

Of course it's a bug where the custom assembly resolver I have resolved this combination of .dlls - normally there shouldn't be a cycle.

But I thought of how to defend against a StackOverflow here and there doesn't seem to be a way to defend against reentrancy on the consumer side.

Ideally I'd place a circuit breaker here:

cecil/Mono.Cecil/ModuleDefinition.cs

Line 741 in 6edabe0

return MetadataResolver.Resolve (type);

Remember the type I'm about to start resolving by adding it to the list, and remove it at the end of the method in a finally block. At the beginning of the method, check if the type is already in the list, and if so, throw an InvalidOperationException("recursive type references") or something.

I know it's kind of an obscure edge case but something to think about.

[KirillOsenkov]

If you think this is worth pursuing I can send a PR.

[KirillOsenkov]

@jbevain here's an attempt to break the circularity of type forwarders:
#806

[KirillOsenkov]

@jbevain we need to fix this, I ran into this again :( Every time I investigate this I forget I already saw this and it takes me an hour or so.

This time a loop for System.Security.Cryptography.ProtectedData:
System.Security.dll -> {System.Security.Cryptography.ProtectedData, Version=0.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a}
{System.Security.Cryptography.ProtectedData, Version=4.0.5.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a}
-> {System.Security, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a}

[KirillOsenkov]

Another cycle between System.Configuration.dll and System.Configuration.ConfigurationManager.dll

[KirillOsenkov]

I revived #806 and added a test. Should be ready to go!