1

So, I have a class

class Room {
    public:
        Room();
        ~Room();
        tile::Tile* tile[11][7]; // Owned
}

The has a constructor and destructor, tile::Tile is an abstract base class, so is a pointer. The array of pointers tile, need to be populated in the constructor like this.

Room::Room() {
    for (std::size_t i = 0; i < 11; ++i) {
        for (std::size_t j = 0; j < 7; ++j) {
            this->tile[i][j] = new tile::Empty();
        }
    }
}

From my understanding, I should also delete these in Room's destructor.

Room::~Room() {
    for (std::size_t i = 0; i < 11; ++i) {
        for (std::size_t j = 0; j < 7; ++j) {
            delete this->tile[i][j];
        }
    }
}

However, doing this results in a return code of 0xc0000374, which is a heap corruption error. Why is this corruption error happening?

Minimum example

class Tile {};

class Empty: public Tile {
    public:
        Empty() {}
};

class Room {
    public:
        Tile* tiles[5];
        Room() {
            for (int i = 0; i < 5; ++i) {
                tiles[i] = new Empty();
            }
        }
        ~Room() {
            for (int i = 0; i < 5; ++i) {
                delete tiles[i];
            }
        }
};

class Maze {
    public:
        Room rooms[5];
        Maze() {
            for (int i = 0; i < 5; ++i) {
                rooms[i] = Room();
            }
        }
};

int main() {
    Maze maze = Maze();
}
trincot
  • 317,000
  • 35
  • 244
  • 286
L. L. Blumire
  • 345
  • 1
  • 2
  • 9
  • 1
    You have corrupted the heap somewhere between the creation and the destruction. It's impossible to guess where or what you did. – molbdnilo Apr 15 '17 at 16:31
  • what is `tile::Empty()`? – SomeWittyUsername Apr 15 '17 at 16:33
  • 1
    As @molbdnilo says the error is elsewhere. Turn the above code into a [mcve] and see if it still happens. – Richard Critten Apr 15 '17 at 16:34
  • `//owned` is not how you express ownership in C++ because compilers don't read comments. Use `std::unique_ptr tile[11][7];` instead so the compiler understands your code and can help you produce the correct behavior. – nwp Apr 15 '17 at 16:36
  • Any advice for how to track down memory corruption. I can post my whole codebase on github and just link to that, because as far as I can tell there shouldn't be anything – L. L. Blumire Apr 15 '17 at 16:39
  • @nwp the comment is for my own purposes. I am using C++03 so do not have access to std::unique_ptr – L. L. Blumire Apr 15 '17 at 16:39
  • 1
    You should probably read [How to debug heap corruption errors](https://stackoverflow.com/questions/1010106/how-to-debug-heap-corruption-errors). – nwp Apr 15 '17 at 16:46
  • It's definitely somewhere else. Comment all the rest of the code out and bring it back in one piece at a time until the error comes back. The last thing you added in is probably the trouble. Take other bits out until you're left with a minimal program that is still wrong. If that isn't a "D'oh" moment come back... – Persixty Apr 15 '17 at 16:55

2 Answers2

0

If this is all the code then it looks ok. I assume you have some more code that deletes some of these entries before the destructor.

In any case you must assign a NULL to pointers right after deletion so additional calls to delete will not raise exception.

If there is other code that did call delete before the destructor and did not assign a NULL to the pointer(s) then this will explain the exception.

PazO
  • 1,314
  • 1
  • 11
  • 30
0

Alright, so the problem for my code was actually in the construction of the Room object. I had a loop intializing a 5 by 5 array with the default constructor (i.e. rooms[i][j] = Room()). Removing this (as C++ will automatically use the default constructor for arrays) solved the problem!

class Tile {};

class Empty: public Tile {
    public:
        Empty() {}
};

class Room {
    public:
        Tile* tiles[5];
        Room() {
            for (int i = 0; i < 5; ++i) {
                tiles[i] = new Empty();
            }
        }
        ~Room() {
            for (int i = 0; i < 5; ++i) {
                delete tiles[i];
            }
        }
};

class Maze {
    public:
        Room rooms[5];
        Maze() {
            //for (int i = 0; i < 5; ++i) {
            //  rooms[i] = Room();
            //}
        }
};

int main() {
    Maze maze = Maze();
}
L. L. Blumire
  • 345
  • 1
  • 2
  • 9
  • There still something wrong with your code. If copy ctor is used in your program, you will end double deleting the Tile*. Google for "rule of three". – Gian Paolo Apr 15 '17 at 20:01
  • That does not change the problem. If your program uses somehow the default generated copy ctor, the 2 instances (the old and the copied one) will share the same Tile pointers and will both call delete on them. This will probably create a memory corrupted scenario, with errors that can arise in any different context – Gian Paolo Apr 16 '17 at 18:47
  • Why does the above code not cause a runtime error then? – L. L. Blumire Apr 16 '17 at 19:28
  • Because probably, you are not calling copy ctor. Or even if you are calling it, the second delete does not free any memory allocated by something else. But you are just setting up a crash for a future modification in your code – Gian Paolo Apr 16 '17 at 19:45
  • What would your solution be then – L. L. Blumire Apr 17 '17 at 08:35