0

So, I'm curious about this thing I can't figure out.

I'm creating some new objects and passing them to a function which stores them in a std::stack.

However, when i want to delete them - they do not actually get deleted, and as such, memory usage will proceed to climb "forever" with my test loop.

Why?

bool StateMachine::changeState(BaseState *state) {
    if (state == nullptr) {
        delete states.top();
        states.pop();
        if (states.size() == 0) {
            return false;
        }
    } else if (state != states.top()) {
        states.push(state);
    }
    return true;
}

Test loop:

while (true) {
    machine.changeState(new MenuState);
    machine.changeState(nullptr);
}

Using a std::unique_ptr instead of raw works, and now ram usage is constant, but still - I wanna know.

Cheers!

ludolover
  • 21
  • 6
  • You have undefined behavior. If you call `states.top()` on an empty `std::stack` you get undefined behavior. – Galik Apr 22 '17 at 00:39
  • The stack is populated during init, so it's never empty. Should probably have mentioned that. – ludolover Apr 22 '17 at 00:43
  • 1
    I tried it locally with a slightly modified version (taking `int*` instead of `BaseState*` + checking for empty stack), and there I'm leaking no memory (ran with address sanitizer). Is it possible that the destructor of `BaseState` is not declared `virtual`? – Corristo Apr 22 '17 at 00:59
  • Offtopic: Btw, the check `state != state.top()` is most likely not what you want. It is checking for equality of the *addresses*, not of the actual state *objects*. So `machine.changeState(new MenuState{}); machine.changeState(new MenuState{});` will lead to two different `MenuState` objects to be on your stack on top of one another. If this is not the behavior you want (which I assume), then you need a ["virtual" `operator==`](http://stackoverflow.com/a/26070512/3601321). – Corristo Apr 22 '17 at 01:15
  • @Corristo Correct! It was missing virtual on the destructor. I guess it working with unique_ptr had me not check elsewhere. Did not know that it behaved differently with derived classes. Learned a new thing today, thanks! – ludolover Apr 22 '17 at 01:18
  • @Corristo Yeah, I'm aware about the last thing you mentioned – ludolover Apr 22 '17 at 01:35

1 Answers1

1

Your code should be correct given the preconditions you've mentioned, but notice that you can allocate and delete objects without reclaiming operating system allocated memory, especially if you leave allocation holes in memory. So check both if memory starts growing then stops and for memory leaks elsewhere, like inside BaseState.

If you're in doubt about preconditions, add an else clause in your if and print something. I should never happen, but if it does, then there may be some problem calling states.top().

trollkill
  • 44
  • 3