Roslyn: Stack Overflow on 1024+ fluent chained call

[eduelias]

Version Used:
csc bundled with Visual Studio 2015 Professional Edition

Steps to Reproduce:

1. Create a fluent chained call like SomeClass.AddProperty(new Property() {...})
2. Chain 1024 or more .AddProperty(new Property() {...}) calls
3. Try to compile

Expected Behavior:
Compile without casting Stack Overflow

Actual Behavior:
Throws Stack Overflow on compiler

[bbarry]

Can confirm I am able to crash VS2015 with the following program:

class Foo {
    private IEnumerable<Property> _properties;
    public IReadOnlyCollection<Property> Properties => new ReadOnlyCollection<Property>((_properties ?? Enumerable.Empty<Property>()).ToList());

    public Foo AddProperty(Property p) {
        return new Foo {_properties = (_properties ?? Enumerable.Empty<Property>()).Concat(new []{p})};
    }
}

internal class Property {}

class Program {
    static void Main() {
        var someclass = new Foo();
        var s2 = someclass
            .AddProperty(new Property())
            .AddProperty(new Property())
            ...;
        Console.WriteLine(s2.Properties.Count());
    }
}

by repeatedly selecting and doubling the .AddProperty(new Property()) lines (select from end of line var s2 = someclass to just before semi, ctrl+c, right arrow, ctrl+v). My installation becomes slow at 10 doublings (1024 calls) and crashes on the 11th. 1024 calls compiles and executes fine for me though (VS2015 Pro Update 1).

Edit: compilation fails at 1090 calls for me in release mode for this program.

[DavidArno]

If ever there was a need for a `CS9999 - WT*&$ are you doing!?!!" compilation error, it would be here...

[suicvne]

lol

[nzall]

I'm curious: what does the spec say about the max length a method chain may have? And is it even sensible to have the possibility of a method chain that's thousands of links long?

@eduelias How did you come across this bug? Did you just do it to see what would happen? Or have you actually come across this bug while working on a real-world project?

[niemyjski]

I wonder if these two are related: https://github.com/dotnet/roslyn/issues/14617

[niemyjski]

I'm also kind of seeing this behavior but with very large amounts of constant cpu usage with a ton of fluent chained calls.

[svick]

@nzall

I'm curious: what does the spec say about the max length a method chain may have?

As far as I can tell, it does not say anything about it. Though it is usually understood that an implementation will have some limits.

And is it even sensible to have the possibility of a method chain that's thousands of links long?

I think it's sensible to expect that the compiler does not crash.

[niemyjski]

I'm seeing this in our real world project: https://github.com/exceptionless/Exceptionless/blob/master/src/Exceptionless.Core/Repositories/Configuration/Indexes/EventIndex.cs This is very common when using elasticsearch fluent mappings or automapper configuration etc.. This should be top priority.

[nzall]

@niemyjski Okay, if real projects are being affected by the bug, then it should indeed be fixed. I'll leave determining importance to the people who have to fix it.

In the meantime, are there any workarounds? For example, could you split up the method chain in smaller sections so the compiler doesn't choke on it? Or would those splits be optimized away by Roslyn?

[niemyjski]

I was just thinking that myself (converting some of the chains to methods, seems like the compiler would optimize that).

[TyOverby]

@nzall @niemyjski Splitting the fluent chain up into smaller sections is actually a valid workaround. In a small test inspired by @bbarry's repro, I found that property chains in groups of 100 _don't_ crash the compiler, while leaving them ungrouped does trigger the crash.

Basically, if necessary, you can turn this:

var result = myobj.SomeMethod().SomeMethod().SomeMethod().SomeMethod();

into this

var temp = myobj.SomeMethod().SomeMethod();
var result = temp.SomeMethod().SomeMethod();

and you can avoid the crash.

repro generation

for i in range(0,10):
     print("s2");
     for k in range(0, 100):
             print(".AddProperty(new Property())");
     print(";");

[eduelias]

@nzall We found it at a real project. We have a migrator that checks our database configuration and creates configuration's classes in C# typed objects. If there's a custom object with a lot of properties or fields, the migrator will generate thousands of chained '.AddProperty()' calls.

We have split the calls into single-calls and fixed our migrator to split when generating classes.

We only noted it when upgrading from Vs 2013 to 2015. The file containing the thousand chained calls were on our project for, at least, 1,5 years!

[jcouv]

I've diagnosed a similar issue in a Watson (stackoverflow crash). In my opinion, there is not much the compiler could do.
At best we could try to detect when a stack overflow is about to happen and report a more helpful diagnostic.

[bbarry]

I think we do try to detect on master:

https://github.com/dotnet/roslyn/blob/614299ff83da9959fa07131c6d0ffbc58873b6ae/src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxWalker.cs#L31

At least that is the cause of the exception on tryroslyn when I do what I did above, generating this exception I've cleaned up:

Insufficient stack to continue executing the program safely. This can happen from having too many functions on the call stack or function on the stack using too much stack space. at System.Runtime.CompilerServices.RuntimeHelpers.EnsureSufficientExecutionStack() at StackGuard.EnsureSufficientExecutionStack(Int32 recursionDepth) in StackGuard.cs:line 17
 at CSharpSyntaxWalker.Visit(SyntaxNode node) in CSharpSyntaxWalker.cs:line 31
 at CSharpSyntaxWalker.DefaultVisit(SyntaxNode node) in CSharpSyntaxWalker.cs:line 54 
 at LocalBinderFactory.DefaultVisit(SyntaxNode node) in LocalBinderFactory.cs:line 718
 [[[
    at CSharpSyntaxVisitor.VisitMemberAccessExpression(MemberAccessExpressionSyntax node) in Syntax.xml.Main.Generated.cs:line 1363
    at Syntax.MemberAccessExpressionSyntax.Accept(CSharpSyntaxVisitor visitor) in Syntax.xml.Syntax.Generated.cs:line 1664
    at CSharpSyntaxWalker.Visit(SyntaxNode node) in CSharpSyntaxWalker.cs:line 33
    at CSharpSyntaxWalker.DefaultVisit(SyntaxNode node) in CSharpSyntaxWalker.cs:line 54
    at LocalBinderFactory.DefaultVisit(SyntaxNode node) in LocalBinderFactory.cs:line 718
 ]]]
 at CSharpSyntaxVisitor.VisitInvocationExpression(InvocationExpressionSyntax node) in Syntax.xml.Main.Generated.cs:line 1471
 at Syntax.InvocationExpressionSyntax.Accept(CSharpSyntaxVisitor visitor) in Syntax.xml.Syntax.Generated.cs:line 3282
 at CSharpSyntaxWalker.Visit(SyntaxNode node) in CSharpSyntaxWalker.cs:line 33
 at LocalBinderFactory.VisitVariableDeclarator(VariableDeclaratorSyntax node) in LocalBinderFactory.cs:line 672 
 at Syntax.VariableDeclaratorSyntax.Accept(CSharpSyntaxVisitor visitor) in Syntax.xml.Syntax.Generated.cs:line 7793
 at CSharpSyntaxWalker.Visit(SyntaxNode node) in CSharpSyntaxWalker.cs:line 33
 at LocalBinderFactory.VisitLocalDeclarationStatement(LocalDeclarationStatementSyntax node) in LocalBinderFactory.cs:line 665
 at Syntax.LocalDeclarationStatementSyntax.Accept(CSharpSyntaxVisitor visitor) in Syntax.xml.Syntax.Generated.cs:line 7603
 at CSharpSyntaxWalker.Visit(SyntaxNode node) in CSharpSyntaxWalker.cs:line 33
 at LocalBinderFactory.Visit(CSharpSyntaxNode syntax, Binder enclosing) in LocalBinderFactory.cs:line 44
 at LocalBinderFactory.VisitBlock(BlockSyntax node) in LocalBinderFactory.cs:line 341
 at Syntax.BlockSyntax.Accept(CSharpSyntaxVisitor visitor) in Syntax.xml.Syntax.Generated.cs:line 7289
 at CSharpSyntaxWalker.Visit(SyntaxNode node) in CSharpSyntaxWalker.cs:line 33
 at LocalBinderFactory.Visit(CSharpSyntaxNode syntax, Binder enclosing) in LocalBinderFactory.cs:line 38
 at LocalBinderFactory.BuildMap(Symbol containingMemberOrLambda, SyntaxNode syntax, Binder enclosing, ArrayBuilder`1 methodsWithYields, Func`3 rootBinderAdjusterOpt) in LocalBinderFactory.cs:line 100
 at ExecutableCodeBinder.ComputeBinderMap() in ExecutableCodeBinder.cs:line 66
 at ExecutableCodeBinder.get_MethodSymbolsWithYield() in ExecutableCodeBinder.cs:line 130
 at ExecutableCodeBinder.ValidateIteratorMethods(DiagnosticBag diagnostics) in ExecutableCodeBinder.cs:line 139
 at MethodCompiler.BindMethodBody(MethodSymbol method, TypeCompilationState compilationState, DiagnosticBag diagnostics, ImportChain& importChain, Boolean& originalBodyNested) in MethodCompiler.cs:line 1512
...

This could instead be written as something of a bouncing stack:

CSharpSyntaxWalker.Visit can be instead written something vaguely like this:

public override void Visit(SyntaxNode node)
{
    if (node == null) {
        return;
    }
    var stack = ImmutableStack<CSharpSyntaxNode>.Create((CSharpSyntaxNode)node);
    while(!stack.IsEmpty) {
        SyntaxNode current;
        stack = stack.Pop(out current);
        var asNode = current.AsNode();
        if (asNode != null)
        {
            current.Accept(this, ref stack);
        }
        else
        {
            this.VisitToken(child.AsToken());
        }
    }
}

WriteGreenAcceptMethods can be:

...
WriteLine("    public override void Accept(CSharpSyntaxVisitor visitor, ref ImmutableStack<CSharpSyntaxNode> stack)");
WriteLine("    {");
WriteLine("        visitor.Visit{0}(this, ref stack);", StripPost(node.Name, "Syntax"));
WriteLine("    }");

and WriteGreenVisitor:

    private void WriteGreenVisitor(bool withArgument, bool withResult)
    {
        var nodes = Tree.Types.Where(n => !(n is PredefinedNode)).ToList();

        WriteLine();
        WriteLine("  internal partial class CSharpSyntaxVisitor" + (withResult ? "<" + (withArgument ? "TArgument, " : "") + "TResult>" : ""));
        WriteLine("  {");
        int nWritten = 0;
        for (int i = 0, n = nodes.Count; i < n; i++)
        {
            var node = nodes[i] as Node;
            if (node != null)
            {
                if (nWritten > 0)
                    WriteLine();
                nWritten++;
                WriteLine("    public virtual " + (withResult ? "TResult" : "void") + " Visit{0}({1} node{2}, ref ImmutableStack<CSharpSyntaxNode> stack)", StripPost(node.Name, "Syntax"), node.Name, withArgument ? ", TArgument argument" : "");
                WriteLine("    {");
                WriteLine("      " + (withResult ? "return " : "") + "this.DefaultVisit(node{0}, ref stack);", withArgument ? ", argument" : "");
                WriteLine("    }");
            }
        }
        WriteLine("  }");
    }

and finally back in CSharpSyntaxWalker, DefaultVisit instead had an additional override:

    public override void DefaultVisit(SyntaxNode node, ref ImmutableStack<CSharpSyntaxNode> stack)
    {
        var childCnt = node.ChildNodesAndTokens().Count;
        int i = 0;

        do
        {
            var child = ChildSyntaxList.ItemInternal((CSharpSyntaxNode)node, i);
            i++;
            stack = stack.Push(child);
        } while (i < childCnt);
    }

(and similar changes on the vb path)

It would be a rather involved change and probably really annoying, and might not fix this issue anyway (a similar structure might be necessary for other visitor patterns).

[jcouv]

The trouble is that such stack overflow are not just possible while visiting syntax. In the case I investigated the binding for the 1000+ fluent chain got too deep.

[TyOverby]

@jcouv When I looked at this back in October with @AlekseyTs, we noticed the same thing: as soon as you fix one visitor, another one breaks. (I think I got three visitors deep before giving up)

[jcouv]

Quick update: A mitigation was checked in https://github.com/dotnet/roslyn/pull/16795 (which at least reduces IDE crashes for such code). But there still seems to be an issue with completing the compilation.

[pyDez]

The same problem occurs for us. We are migrating from VS2013 to VS2015. Our main project is containing some methods with 1024+ calls to fluent method. Msbuild process crashes with following output :

Process is terminated due to StackOverflowException.

C:\Program Files (x86)\MSBuild\14.0\bin\Microsoft.CSharp.Core.targets(67,5): error MSB6006: "csc.exe" exited with code -1073741571.

We are using fluent method to create complex domain data structure.

I created a thread on StackOverflow trying to identify the issue.