Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic type resolution goes into infinite recursion #16908

Closed
ScramblerUSA opened this issue Apr 5, 2016 · 10 comments · Fixed by dotnet/corefx#25196
Closed

Dynamic type resolution goes into infinite recursion #16908

ScramblerUSA opened this issue Apr 5, 2016 · 10 comments · Fixed by dotnet/corefx#25196
Assignees
Labels
area-Microsoft.CSharp bug help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@ScramblerUSA
Copy link

Under certain circumstances dynamic type resolution goes into infinite recursion (causing program to terminate with StackOverflowException) alternating between these methods:

Microsoft.CSharp.dll!Microsoft.CSharp.RuntimeBinder.Semantics.TypeManager.SubstTypeCore(Microsoft.CSharp.RuntimeBinder.Semantics.CType type, Microsoft.CSharp.RuntimeBinder.Semantics.SubstContext pctx)
Microsoft.CSharp.dll!Microsoft.CSharp.RuntimeBinder.Semantics.TypeManager.SubstTypeArray(Microsoft.CSharp.RuntimeBinder.Semantics.TypeArray taSrc, Microsoft.CSharp.RuntimeBinder.Semantics.SubstContext pctx)

This happens only when an argument of type "dynamic" is passed, of course. Static typing works fine.

Repro:

using System;
class Program
{
    static void Main(string[] args)
    {
        // vvv--- changing "dynamic" to "var" or "string" here fixes the issue
        dynamic parsedDocument = "";
        var mystery = new FailingClass<string>();
        Console.WriteLine("Entering...");
        mystery.FailingMethod(parsedDocument);
        Console.WriteLine("... and we are back!");
        Console.ReadLine();
    }
}
public abstract class CommonBase<T> {}

// Technically, this class does nothing and deriving from it should be identical to deriving from CommonBase
public abstract class FailingClassBase<T> : CommonBase<T> {}

// However, deriving from CommonBase instead of FailingClassBase here also fixes the issue
// ----------------------------vvvvvvvvvvvvvvvv
public class FailingClass<T> : FailingClassBase<FailingClass<T>>
{
    public void FailingMethod(T src) {}
}

[EDIT] Fixed callstack formatting by @karelz
[EDIT] Inlined source code from original attachment repro.txt, with minor formatting changes by @karelz

@ContentsMayVary
Copy link

ContentsMayVary commented Oct 5, 2016

Here's another repro - this also causes StackOverflowException:

class First<T> where T : First<T> { }
class Second<T> : First<T> where T : First<T> { }
class Third<T> : Second<Third<T>> { }

class Program
{
    static void Main()
    {
        dynamic x = new Third<int>();
        Print(x); // Uncomment this and the exception goes away.
    }

    static void Print(object obj) { }
}

Also see:

http://stackoverflow.com/questions/39870305/system-stackoverflowexception-error-when-try-to-get-a-dynamic-proppery-from-an#comment67032482_39870305
http://stackoverflow.com/questions/22672775/stackoverflowexception-when-accessing-member-of-generic-type-via-dynamic-net-c

[EDIT] Add C# syntax highlighting by @karelz

@karelz
Copy link
Member

karelz commented Nov 28, 2016

We need someone to get it under debugger and find out what is wrong in .NET Core code.

@JonHanna
Copy link
Contributor

The loop is straight-forward enough. In the last of the examples produced, for example, it trys to substitute the types in Third<T> to get Third<int> then work out the base when is a type of Second<T> typed with Third<T> which has a base of Second<T> typed with Third<T> which has a base…

Breaking the loop does not seem to be easy though.

@karelz
Copy link
Member

karelz commented Aug 31, 2017

There is duplicate report in dotnet/corefx#23706 with variant of the same bug: Second<T>=BuilderBaseEx<T>, but the constraint is slightly (and likely insignificantly) different:

Note: The original (previously "hidden") repro does not have any constraints on generic parameters.

JonHanna referenced this issue in JonHanna/corefx Nov 11, 2017
Push logic for calculating the substitution necessary on base types
when type or the base are generic into AggregateType and defer it until
first call.

This allows cycling chains of derivation and type parameters.

Fixes #7527
JonHanna referenced this issue in JonHanna/corefx Nov 11, 2017
Confirms fix for #7527 also fixes #23706
@JonHanna
Copy link
Contributor

The great thing about not having time to contribute to corefx for a month, is getting a fresher look at some things. I'd tried a variant of the fix in dotnet/corefx#25196 before but must have slipped up, since it went quite smoothly this time.

JonHanna referenced this issue in JonHanna/corefx Nov 11, 2017
Push logic for calculating the substitution necessary on base types
when type or the base are generic into AggregateType and defer it until
first call.

This allows cycling chains of derivation and type parameters.

Fixes #7527
JonHanna referenced this issue in JonHanna/corefx Nov 11, 2017
Confirms fix for #7527 also fixes #23706
JonHanna referenced this issue in JonHanna/corefx Nov 15, 2017
Push logic for calculating the substitution necessary on base types
when type or the base are generic into AggregateType and defer it until
first call.

This allows cycling chains of derivation and type parameters.

Fixes #7527
JonHanna referenced this issue in JonHanna/corefx Nov 15, 2017
Confirms fix for #7527 also fixes #23706
JonHanna referenced this issue in JonHanna/corefx Nov 20, 2017
Push logic for calculating the substitution necessary on base types
when type or the base are generic into AggregateType and defer it until
first call.

This allows cycling chains of derivation and type parameters.

Fixes #7527
JonHanna referenced this issue in JonHanna/corefx Nov 20, 2017
Confirms fix for #7527 also fixes #23706
VSadov referenced this issue in dotnet/corefx Nov 21, 2017
…#25196)

* Defer calculation of AggregateType.GetBaseClass()

Push logic for calculating the substitution necessary on base types
when type or the base are generic into AggregateType and defer it until
first call.

This allows cycling chains of derivation and type parameters.

Fixes #7527

* Add test for variant of cyclic type definitions.

Confirms fix for #7527 also fixes #23706

* Overrides SubstType and SubstTypeCore for AggregateType when used

In one case replaces the equivalent existing override completely.

Allows type-checking and casting to be avoided.

* Test another variant of #7527

Based on https://stackoverflow.com/q/19612325/400547
@JonHanna
Copy link
Contributor

It might be worth making this netfx-port-consider, since there are several stackoverflow questions about it with netfx, suggesting it affects a fair number of people.

@karelz
Copy link
Member

karelz commented Nov 21, 2017

@JonHanna did you see more than the 2 StackOverflow links above? If there are any highly upvoted, that would be worth linking from here. It will be useful at the point we do the port to assess need vs. risk. Thanks!

@JonHanna
Copy link
Contributor

JonHanna commented Nov 21, 2017

There was also https://stackoverflow.com/q/19612325/400547 that gave me the idea of testing it in cases where it should throw RBE.

@karelz
Copy link
Member

karelz commented Nov 21, 2017

Which last one? Can you please edit your reply to keep just the relevant links? Thanks!

@JonHanna
Copy link
Contributor

NP

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.1.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Microsoft.CSharp bug help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants