Evaluating a record with recursive reference results in termination of the program with Stack Overflow

[tmat]

Version Used:
Version 16.9.0 Preview 1.0 [30614.203.main]

Steps to Reproduce:

Place breakpoint as indicated, F5 and open Locals Window, then step (F10).

record C
{
    public int X { get; init; }
    public int Y { get; init; }
    public int Z { get; init; }

    public C Inner { get; set; }
}

class Program
{
    static void Main(string[] args)
    {
        var c = new C();
        c.Inner = c; // breakpoint here
    }
}

Actual Behavior:

Expected Behavior:

SO is prevented.

Perhaps the implementation should only go to certain depth and then bail.

[RikkiGibson]

Tagging @jcouv

[jcouv]

That was discussed in LDM and is by-design. The ToString, GetHashCode and Equals implementations recurse without limit.
We recommend adding a debugger display or customizing ToString for such scenarios.

[tmat]

What was the reasoning for this design decision? Is the scenario deemed unlikely? Have you considered that the VS debugger evaluates ToString when the user hovers on a variable to show a data tip with the value of the variable? Hard crashing the debuggee process when hovering over source code is not a great experience. Also, customizing the record implementation might not be an option when debugging third party code.

Limiting the stack depth of ToString to some reasonable number (say 64) would be easy to implement. I believe other languages (e.g. Python and Ruby) do that in their object inspection methods.

If it is not possible to do so we could special case records in EE and not call ToString on them.

[PathogenDavid]

What was the reasoning for this design decision?

Here's the full LDM meeting notes from when this was discussed.

[CyrusNajmabadi]

Assigning to myself to proceed with a WG to come up with a proposal here.

[CyrusNajmabadi]

TAgging @cston @333fred @RikkiGibson @jcouv who all showed interest in this topic.

[RikkiGibson]

Resolving this issue as the scenario described is now fixed. We now throw InsufficientExecutionStackException instead of actually overflowing the stack.