Stack overflow with target framework `net48` and `Polly.Core`

[peterhirn]

Reproduce

dotnet new classlib

dotnet tool install --create-manifest-if-needed dotnet-ilrepack

Edit .csproj setting target framework to net48 and add LangVersion

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net48</TargetFramework>
    <RootNamespace>repack_2</RootNamespace>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <LangVersion>12.0</LangVersion>
  </PropertyGroup>
</Project>

dotnet add package Polly.Core

dotnet publish -c Release

dotnet ilrepack /internalize /parallel /verbose /lib:bin/Release/net48/publish/ /out:out.dll bin/Release/net48/publish/repack-2.dll bin/Release/net48/publish/Polly.Core.dll 2>&1|less

Fails with

...
- Fixing references for type Polly.Timeout.TimeoutGeneratorArguments
- Fixing references for type Polly.Timeout.TimeoutRejectedException
- Fixing references for type Polly.Timeout.TimeoutResilienceStrategy
- Fixing references for type Polly.Timeout.TimeoutStrategyOptions
Stack overflow.
   at Mono.Cecil.MemberReference.get_DeclaringType()
   at Mono.Cecil.TypeReference.get_DeclaringType()
   at Mono.Cecil.TypeReference.get_IsNested()
   at Mono.Cecil.MetadataResolver.GetTypeDefinition(Mono.Cecil.ModuleDefinition, Mono.Cecil.TypeReference)
   at Mono.Cecil.MetadataResolver.GetType(Mono.Cecil.ModuleDefinition, Mono.Cecil.TypeReference)
   at Mono.Cecil.MetadataResolver.Resolve(Mono.Cecil.TypeReference)
   at Mono.Cecil.ModuleDefinition.Resolve(Mono.Cecil.TypeReference)
   at Mono.Cecil.ExportedType.Resolve()
   at Mono.Cecil.MetadataResolver.GetType(Mono.Cecil.ModuleDefinition, Mono.Cecil.TypeReference)
   at Mono.Cecil.MetadataResolver.Resolve(Mono.Cecil.TypeReference)
   at Mono.Cecil.ModuleDefinition.Resolve(Mono.Cecil.TypeReference)
   at Mono.Cecil.ExportedType.Resolve()
...

Notes

• dotnet version 9.0.203
• works with target framework netstandard2.0

[KirillOsenkov]

Thanks for the detailed bug report, this made it easy to investigate and fix.

Published 2.0.42

[peterhirn]

@KirillOsenkov Thanks for the extremely fast fix ❤️

Unfortunately I found another way to create a stack overflow, given the same project setup but additionally merging System.ComponentModel.Annotations.dll 😬

dotnet ilrepack --version
2.0.42+f7d83c4380a6a323da8c82eb311def3b5402bdee

dotnet ilrepack /internalize /parallel /verbose /lib:bin/Release/net48/publish/ /out:out.dll bin/Release/net48/publish/repack-2.dll bin/Release/net48/publish/Polly.Core.dll bin/Release/net48/publish/System.ComponentModel.Annotations.dll 2>&1|less

- Fixing references for type Polly.Timeout.TimeoutStrategyOptions
Resolved 'System.ComponentModel.Annotations, Version=0.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' from '/home/peter/Projects/test/dotnet/repack-2/bin/Release/net48/publish/System.ComponentModel.Annotations.dll'
Stack overflow.
   at Mono.Cecil.Metadata.Row`2[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7c
ec85d7bea7798e]]..ctor(System.__Canon, System.__Canon)
   at Mono.Cecil.TypeDefinitionCollection.GetType(System.String, System.String)
   at Mono.Cecil.ModuleDefinition.GetType(System.String, System.String)
   at Mono.Cecil.MetadataResolver.GetTypeDefinition(Mono.Cecil.ModuleDefinition, Mono.Cecil.TypeReference)
   at Mono.Cecil.MetadataResolver.GetType(Mono.Cecil.ModuleDefinition, Mono.Cecil.TypeReference)
   at Mono.Cecil.MetadataResolver.Resolve(Mono.Cecil.TypeReference)
   at Mono.Cecil.ModuleDefinition.Resolve(Mono.Cecil.TypeReference)
   at Mono.Cecil.ExportedType.Resolve()
   at Mono.Cecil.MetadataResolver.GetType(Mono.Cecil.ModuleDefinition, Mono.Cecil.TypeReference)
   at Mono.Cecil.MetadataResolver.Resolve(Mono.Cecil.TypeReference)
   at Mono.Cecil.ModuleDefinition.Resolve(Mono.Cecil.TypeReference)
   at Mono.Cecil.ExportedType.Resolve()

[KirillOsenkov]

I took another look. This issue is far more complicated than I thought originally.

The whole logic of assembly resolution in ILRepack is wrong.

This is rooted in the fact that Mono.Cecil resolution resolves from Desktop if it's netfx Cecil, but it resolves from Core if it's Core cecil:
https://github.com/jbevain/cecil/blob/3136847ea620fb9b4a3ff96bc4f573148e8bd2e4/Mono.Cecil/BaseAssemblyResolver.cs#L136-L165

The proper fix is to stop using the Cecil resolver altogether and write a new assembly resolver that can resolve from both Core and Framework regardless of the target of Cecil itself.

I happen to have a more advanced resolver implemented here:
https://github.com/KirillOsenkov/MetadataTools/blob/main/src/BinaryCompatChecker/Assemblies.cs
and
https://github.com/KirillOsenkov/MetadataTools/blob/main/src/BinaryCompatChecker/Framework.cs

However it will be a non-trivial amount of work to switch ILRepack to use this resolver. I don't have the time or resources to do it in the near future.

The problem is that you're using the ILRepack tool which targets .NET Core, and so it uses the Cecil that was compiled for Core, while the assemblies you're repacking are targeting netfx.

As a workaround, stop using the .NET tool, and use the ILRepack NuGet package, which targets desktop netfx. For now it seems like a known limitation that you need to use the desktop ILRepack to repack desktop assemblies, and use the dotnet global tool to repack Core assemblies.

[peterhirn]

Thanks for the thorough investigation and explanation.

Your workaround resolves my issue:

dotnet add package ILRepack

/home/peter/.nuget/packages/ilrepack/2.0.42/tools/ILRepack.exe /internalize /parallel /verbose /lib:bin/Release/net48/publish/ /out:out.dll bin/Release/net48/publish/repack-2.dll bin/Release/net48/publish/Polly.Core.dll bin/Release/net48/publish/System.ComponentModel.Annotations.dll

I actually tried this workaround previously, but made the mistake calling ILRepack.exe with dotnet, ie.

dotnet /home/peter/.nuget/packages/ilrepack/2.0.42/tools/ILRepack.exe ...

In this case ilrepack gets stuck (using 100% of one core) when merging System.ComponentModel.Annotations.dll.

Overall I think building framework assemblies with core is a niche usecase. The projects which affect me did already move to core and I only need to support the framework assemblies for outdated versions which will reach eol in a couple of years.

I'm going to close this issue. Feel free to reopen, or convert to a discussion.

Thanks again for your work! ❤️