Skip to content
This repository has been archived by the owner on Nov 1, 2020. It is now read-only.

Npgsql causes compilation to fail or infinite loop #6052

Closed
benaadams opened this issue Jul 4, 2018 · 9 comments
Closed

Npgsql causes compilation to fail or infinite loop #6052

benaadams opened this issue Jul 4, 2018 · 9 comments

Comments

@benaadams
Copy link
Member

Npgsql 4.0.1 https://www.nuget.org/packages/Npgsql/ and using the NpgsqlFactory.Instance property causes compilation to fail

Seems to be some type recursion?

EXEC : error : [TEMPORARY EXCEPTION MESSAGE] ClassLoadValueClassTooLarge:
NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<
 NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<
 NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<
 NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<
 NpgsqlTypes.NpgsqlRange`1<System.String>>>>>>>>>>>>>
, Npgsql, Version=4.0.1.0, Culture=neutral, PublicKeyToken=5d8b90d52f46fda7 [C:\GitHub\FrameworkBenchmarks\frameworks\CSharp\aspnetcore-corert\PlatformBenchmarks\PlatformBenchmarks.csproj]
  Internal.TypeSystem.TypeSystemException+TypeLoadException: [TEMPORARY EXCEPTION MESSAGE] ClassLoadValueClassTooLarge: NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<NpgsqlTypes.NpgsqlRange`1<System.String>>>>>>>>>>>>>, Npgsql, Version=4.0.1.0, Culture=neutral, PublicKeyToken=5d8b90d52f46fda7
     at Internal.TypeSystem.ThrowHelper.ThrowTypeLoadException(ExceptionStringID id, String typeName, String assemblyName)
     at ILCompiler.DependencyAnalysis.EETypeNode.CheckCanGenerateEEType(NodeFactory factory, TypeDesc type)
     at ILCompiler.DependencyAnalysis.NodeFactory.<CreateNodeCaches>b__36_1(TypeDesc type)
     at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
     at ILCompiler.DependencyAnalysis.ReadyToRunGenericHelperNode.InstantiateDependencies(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation)
     at ILCompiler.DependencyAnalysis.ShadowConcreteMethodNode.GetStaticDependencies(NodeFactory factory)
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.GetStaticDependenciesImpl(DependencyNodeCore`1 node)
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ComputeMarkedNodes()
     at ILCompiler.ILScanner.ILCompiler.IILScanner.Scan()
     at ILCompiler.Program.Run(String[] args)
     at ILCompiler.Program.Main(String[] args)

Including it in the rd.xml

<Assembly Name="Npgsql" Dynamic="Required All" />

Instead seems like it infinite loops the compiler causing ever growing memory usage (killed it when if hit 5GB)

@roji
Copy link
Member

roji commented Jul 4, 2018

Ouch... Any idea on what Npgsql may be doing to trigger this? How does corert "discover" a type such as NpgsqlRange<NpgsqlRange<...>>?

@YohDeadfall
Copy link

@roji We already have the same issue in our repo (npgsql/npgsql#1861) and it's not Npgsql related.

@roji
Copy link
Member

roji commented Jul 4, 2018

Right, I thought this sounded familiar... I'm still wondering if there's any weird/specific Npgsql code that's triggering this issue (even if it's a UWP / cortrt issue).

Will close npgsql/npgsql#2040 at least for now but will watch this issue. If there's any questionable code on Npgsql's side please let me know and I'll take a look.

@MichalStrehovsky
Copy link
Member

Yeah, this is generic recursion. What's happening is pretty much this:

class Program
{
    struct Gen<T>
    {
        private T Val1;
        private T Val2;
    }

    static void Recurse<T>(int x)
    {
        if (x != 0)
            Recurse<Gen<T>>(x - 1);
    }

    static void Main()
    {
        Recurse<int>(3);
    }
}

If we're in an environment that has a JIT, we'll end up just-in-time compiling method Recurse into machine code 3 times when the program is launched (instantiated over int, Gen<int>, Gen<Gen<int>>.

But an AOT compiler is pretty much screwed. Since in general we can't prove how many times the instantiation will happen, it will end up compiling forever, or until it runs out of memory/stack/or Gen becomes to be too big and we find out in a place we can't recover from (which was your case).

What the compiler needs to do is:

  • Detect that this is happening (we track that in Detect infinite generic expansion #363)
  • And either:
    1. Fail the compilation with a saner error message (which is the only thing the above issue is tracking), or
    2. Generate a universal method body for Recurse. This would be code that is not particularly efficient (definitely much worse than the JITted code), but at least works for any T. Universal method bodies are a big workitem and we don't currently have a roadmap.

The UWP AOT compiler is a bit more mature and we actually fixed this in .NET Native in our 2.1 release. The fix should be available in version 6.1.5 of the Microsoft.NETCore.UniversalWindowsPlatform package.

@MichalStrehovsky
Copy link
Member

MichalStrehovsky commented Jul 4, 2018

I actually wrote a regression test for the specific case of recursion in npgsql when we were doing the fix in .NET Native for UWP apps. The pattern is of course way more convoluted.

If you can break the recursion there, it would fix the CoreRT compiler issue.

#pragma warning disable 169

struct SqlRange<T>
{
    private T Value1;
    private T Value2;
}

class ChunkingTypeHandler<T> : TypeHandler<T>
{
}

class RangeHandler<T> : ChunkingTypeHandler<SqlRange<T>>
{
    public RangeHandler()
    {
        new TypeHandler<T>().ToString();
    }
}

class TypeHandler
{
    public virtual void CreateRangeHandler() { }
}

class TypeHandler<T> : TypeHandler
{
    public override void CreateRangeHandler()
    {
        new RangeHandler<T>().ToString();
    }
}

internal class Program
{
    private static void Main(string[] args)
    {
        new TypeHandler<bool>().CreateRangeHandler();
    }
}

@roji
Copy link
Member

roji commented Jul 4, 2018

Ok, I'll try to take a look on the Npgsql side, maybe the type hierarchy can be factored to avoid this.

@roji
Copy link
Member

roji commented Jul 4, 2018

FYI have just submitted PR npgsql/npgsql#2042 which refactors Npgsql's internal type hierarchy to avoid this. @benaadams (or @MichalStrehovsky), can you please test nupkg 4.1.0-ci.1111+sha.8a9e6c98b from Npgsql's unstable myget feed and confirm that the issue is gone?

@YohDeadfall
Copy link

@benaadams I've merged npgsql/npgsql#2060 to fix the issue, but you need to write a rd.xml file yourself to use Npgsql, since we use reflection in the connection string builder and other places too. Later we will provide it from the box.

@MichalStrehovsky
Copy link
Member

@YohDeadfall @roji Thank you for making the fix! We'll keep tracking the issue with infinite generic expansion for CoreRT in #363, but I think we can close this now.

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

4 participants