StackOverflowException on Getter C#

Question

I am getting a StackOverflowException on the get; of a property in an abstract class.

public abstract class SenseHatSnake
    {

        private readonly ManualResetEventSlim _waitEvent = new ManualResetEventSlim(false);

        protected SenseHatSnake(ISenseHat senseHat)
        {
            SenseHat = senseHat;
        }

        protected static ISenseHat SenseHat { get; set; } // This Line

        public virtual void Run()
        {
            throw new NotImplementedException();
        }

        protected void Sleep(TimeSpan duration)
        {
            _waitEvent.Wait(duration);
        }

    }

I am setting and getting it here:

public class SnakeGame : SenseHatSnake
{
    private readonly int _gameSpeed = 1000;
    private static Timer _updatePositionTimer;
    private bool _gameOver = false;

    public readonly Movement Movement = new Movement(SenseHat);
    public readonly Food Food = new Food(SenseHat);
    public readonly Body Body = new Body(SenseHat);
    public readonly Display Display = new Display(SenseHat);
    public readonly Draw Draw = new Draw(SenseHat);

    public SnakeGame(ISenseHat senseHat)
        : base(senseHat)
    {
    }
    //More code
}

One of those classes look like this:

public class Movement : SnakeGame
{

    public Movement(ISenseHat senseHat)
        : base(senseHat)
    {
    }
    //More code
}

To my knowledge a StackOverflowException means I somewhere have an an infinite loop or infinite recursion, but I honestly don't know where, nor do I know how to solve it.

best way to diagnose this is debugging the code and checking call stack. — Selman Genç
– Selman Genç, Commented Aug 30, 2018 at 22:40
You need to make a minimal reproducible example and narrow down the problem. — mason
– mason, Commented Aug 30, 2018 at 22:41
@SelmanGenç I can't. I am deploying on a raspberry Pi, and this is the only information I get. — Luuk Wuijster
– Luuk Wuijster, Commented Aug 30, 2018 at 22:42
@LuukWuijster so you can't just copy this code in a console application and debug it? — Selman Genç
– Selman Genç, Commented Aug 30, 2018 at 22:57
@Luuk: I disagree, creating an MVC is definitely possible. For demonstrative purposes, I've created a Complete and Verifiable example at gist.github.com/briangithex/e4dacd3b58fb675a7be8dce56d8a74a2 . I intentionally omitted making it minimal in order to make it easier to recognize how I created a complete example using your posted code. Anyhow, personally I recommend investigating problems in difficult-to-simplify problems using binary search for bug. If you paste my code and yours into a diff tool, you'll note that the middle section is identical. — Brian
– Brian, Commented Aug 31, 2018 at 22:39

Bagdan Imr · Accepted Answer · 2018-08-30 23:19:49Z

13

This line in SnakeGame causes a recursion

public readonly Movement Movement = new Movement(SenseHat);

Since Movement is inherited from SnakeGame, its constructor will initialize SnakeGame, calling the line above again to initialize its own Movement field. That results into recursion.

edited Aug 30, 2018 at 23:19

answered Aug 30, 2018 at 22:46

Bagdan Imr

1,2861 gold badge11 silver badges18 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Jonathon Chase Over a year ago

I don't know why this answer was down voted, it is correct that this is the source of the stack overflow.

Olivier Jacot-Descombes Over a year ago

I don't quite see why this is a recursion, because new Movement(SenseHat) calls the getter of SenseHat and then after a few nested calls the constructor of SenseHatSnake sets SenseHat calling the setter. These are two different methods.

Selman Genç Over a year ago

@OlivierJacot-Descombes movement's constructor call the SnakeGame's constructor, and while snake game is being initialized code enters Movement = new Movement(SenseHat); again because the fields are initialized in the constructor as well. so the snake's constructor calls this line and this line calls snake's constructor.

Olivier Jacot-Descombes Over a year ago

Okay I see. I was focused on the SenseHat property. It has nothing to do with this property. The Movement constructor is calling itself indirectly. The OP's StackOverflow happened in the the SenseHat property by accident, because the stack was almost full.

Eric Lippert · Accepted Answer · 2018-08-30 22:54:39Z

The origin of the overflow has been identified in other answers, and other answers have pointed out the architectural design problems in your program sketch.

I thought I'd say briefly how to debug such issues yourself.

First: when faced with a stack overflow, there is almost always an unbounded recursion. When the overflow is reported in a member that plainly has no stack overflow, what has happened? This has happened:

void Bad()
{
    Good();
    Bad();
}

If Bad is called then it will stack overflow, but most of the time the overflow will be reported in Good, because Good probably uses more stack than any single call to Bad.

What you need to do is look at the call stack, because it will be Good / Bad / Bad / Bad / Bad ... and that tells you that it is Bad that is doing the unbounded recursion. The key to finding the source of a recursion is to find the thing that is actually calling itself, directly or indirectly. Use the instrumentation at your disposal to do so; the exception contains a trace of the call stack.

Sergey Kalinichenko · Accepted Answer · 2018-08-30 23:19:09Z

2

This is a bad pattern:

protected SenseHatSnake(ISenseHat senseHat)
{
    SenseHat = senseHat;
}

protected static ISenseHat SenseHat { get; set; }
//        ^^^^^^

Your constructor is setting a static field shared among all subclasses of SenseHatSnake, meaning that the last class setting the field "wins". It also means that you can never set this field, because in order to construct the value to assign to the field you must create an object that has to have that field set - a snake chasing its own tail. Also you cannot derive Movement from a class that constructs a member of type Movement as part of its initialization.

Fixing this requires some serious re-organization of your classes:

public class SnakeGame {
    private readonly int _gameSpeed = 1000;
    private static Timer _updatePositionTimer;
    private bool _gameOver = false;

    public Movement Movement {get;}
    public Food Food {get;}
    public Body Body {get;}
    public Display Display {get;}
    public Draw Draw {get;}
    public SnakeGame(ISenseHat senseHat)
    {
        Movement = new Movement(this);
        Food = new Food(this);
        Body = new Body(this);
        Display = new Display(this);
        Draw = new Draw(this);
    }
    //More code
}

public abstract class GameObject {
    protected readonly SnakeGame game;
    protected GameObject(SnakeGame game) {
        this.game = game;
    }
}

public class Movement : GameObject
{
    public Movement(SnakeGame game)
    : base(senseHat)
    {
    }
    //More code
}

Now subclasses of GameObject share SnakeGame object, gaining access to its properties.

edited Aug 30, 2018 at 23:19

answered Aug 30, 2018 at 22:49

Sergey Kalinichenko

729k85 gold badges1.2k silver badges1.6k bronze badges

5 Comments

Luuk Wuijster Over a year ago

Doing that gives a StackOverflow exception on Movement = new Movement(senseHat);

Sergey Kalinichenko Over a year ago

@LuukWuijster It looks like you need to re-think your inheritance strategy: a class in the middle of inheritance hierarchy (SnakeGame) should not be constructing its subclass (Movement) as part of its initialization. If you are inheriting for the sake of sharing fields, you should share the data in an object outside the inheritance hierarchy, and then share that object (i.e. use containment instead of inheritance).

Luuk Wuijster Over a year ago

What I did in the first place was inheriting all classes from SenseHatSnake. However this caused a StackOverflow exception aswell because I needed other classes, and they needed the same class etc. Like so class1{ class2 = new class2() } class2{ class1 = new class1() } What would be a good way to do things like this?

Sergey Kalinichenko Over a year ago

@LuukWuijster Take a look at the edit, this should give you an idea of the kind of restructuring I had in mind.

Luuk Wuijster Over a year ago

Hmmmm... Okay, I see. but I think I will try to use DI instead.

Collectives™ on Stack Overflow

StackOverflowException on Getter C#

3 Answers 3

4 Comments

Comments

5 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

Comments

5 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related