C++ - Object is destroyed before the destructor is executed

Question

I have a class which have a 2D vector of pointers to dynamically allocated instances (wallGameObjects) which I want to delete when the destructor is called, but by the time the destructor executes the vector is already destroyed.

The destructor calls removeMaze():

MazeSceneBuilder::~MazeSceneBuilder()
{
    removeMaze();

    mSceneManager->destroySceneNode(mazeNode);
}

And the object's member variables are already gone:

void MazeSceneBuilder::removeMaze()
{
    // Remove walls
    for (unsigned int x = 0; x < mazeWidth; x++)
    {
        for (unsigned int z = 0; z < mazeWidth; z++)
        {
            if (wallGameObjects[x][z] != nullptr)
            {
                mGameObjectManager->removeGameObject(wallGameObjects[x][z]);
            }
        }
    }

    // remove ground
    mGameObjectManager->removeGameObject(groundGameObject);

    // Destroy collision shapes
    destroyCollisionShapes();
}

Breakpoint on the line "mGameObjectManager->removeGameObject(groundGameObject);" shows this:

this    0x00000000 <NULL>   MazeSceneBuilder *

It doesn't make sense to me. I tried to call "removeMaze()" before the object is destroyed and it worked just fine. Are the object's members suppose to be gone before the destructor executes?

EDIT:

As requested, here's the destructor of MultiplayerMaze:

MultiplayerMaze::~MultiplayerMaze()
{
    destroyPhysicsWorld();
}

destroyPhysicsWorld:

void MultiplayerMaze::destroyPhysicsWorld()
{
    delete dynamicsWorld;
    delete solver;
    delete collisionConfiguration;
    delete dispatcher;
    delete broadphase;
}

EDIT 2:

MultiplayerMaze class definition:

class MultiplayerMaze : public BaseApplication
{
public:
    MultiplayerMaze();
    virtual ~MultiplayerMaze();

protected:
    virtual void createScene();
    virtual bool frameRenderingQueued(const Ogre::FrameEvent& evt);
    virtual bool configure();

    virtual void createPhysicsWorld();
    virtual void destroyPhysicsWorld();

    // Physics members
    btBroadphaseInterface* broadphase;
    btDefaultCollisionConfiguration* collisionConfiguration;
    btCollisionDispatcher* dispatcher;
    btSequentialImpulseConstraintSolver* solver;
    btDiscreteDynamicsWorld* dynamicsWorld;

    GameObjectManager gameObjectManager;

    MazeSceneBuilder mazeSceneBuilder;
    MazeGenerator mazeGenerator;
};

MazeSceneBuilder class definition:

class MazeSceneBuilder
{
protected:
    std::vector<std::vector<bool>> mMaze;
    unsigned int mazeWidth, mazeHeight;
    float mMazeScale;

    GameObjectManager* mGameObjectManager;
    Ogre::SceneManager* mSceneManager;
    Ogre::String mWallEntity;
    Ogre::String mGroundMaterial;
    Ogre::Entity* mGround;
    Ogre::SceneNode* mazeNode;
    btCollisionShape* wallCollisionShape;
    btCollisionShape* collisionPlanes[5];
    GameObject* groundGameObject;
    std::vector<std::vector<GameObject*>> wallGameObjects;

    void setupGround();     // Creates a plane entity using the ground materiala and the maze size.
    void createCollisionShapes();       // Create collision shapes for the wall, ground, and bounding planes.
    void destroyCollisionShapes();
    void DestroyAllAttachedMovableObjects(Ogre::SceneNode* sceneNode);

public:
    MazeSceneBuilder();
    MazeSceneBuilder(GameObjectManager* gameObjectManager);
    ~MazeSceneBuilder();

    void setGameObjectManager(GameObjectManager* gameObjectManager);
    bool setMaze(std::vector<std::vector<bool>> maze);      // Returns false if the provided maze was invalid.
    void setMazeScale(float scale);
    void setWallEntity(Ogre::String entityName);        // Must be a square cuboid of 1x2x1
    void setGroundMaterial(Ogre::String materialName);  // Should be a tiled square texture.
    void buildMaze();
    void updateMaze(const std::vector<std::vector<bool>>& maze);
    void removeMaze();
};

Can you please have a look at the callstack and check where your destructor gets called and if its not automatically called post the line where you invoke the constructor? this **can** be nullptr, but only if you call the constructor by hand on a pointer that is nullptr. — Muepe, Apr 01 '14 at 21:55
You get to choose between a) C++ language implementation flaw, b) cosmic ray particle flipped a RAM bit, c) your code corrupting the heap. a and b are the easier ones to eliminate. — Hans Passant, Apr 01 '14 at 21:56
@Muepe It's being called AFTER the destructor: Multiplayer Maze.exe!MazeSceneBuilder::removeMaze() Line 192 C++ Multiplayer Maze.exe!MazeSceneBuilder::~MazeSceneBuilder() Line 30 C++ I don't call it from anywhere else, so why? — yoni0505, Apr 01 '14 at 22:01
@yoni0505 Not from where removeMaze is called but where does your constructor get invoked from? Do you manually delete your object or is it an automatic destructor from an object going out of scope? — Muepe, Apr 01 '14 at 22:03
Destructors don't just happen. Either 1) an object went out of scope, 2) You are calling `delete ` somewhere, or worse 3) you are explicitly calling the destructor previously, and then the destructor is called again due to the object going out of scope. So look at the call stack and see what is invoking the destructor. — PaulMcKenzie, Apr 01 '14 at 22:05
@muepe The constructor is being called from the constructor of a different class(MultiplayerMaze). And the desctructor is called automatically when the MultiplayerMaze is destroyed since it's a member variable. — yoni0505, Apr 01 '14 at 22:07
Could you please post the destructor of `MultiplayerMaze` in an edit? And preferably how the member looks that holds the `MazeSceneBuilder` getting deleted. — Muepe, Apr 01 '14 at 22:09
@yoni0505 - 1) Maybe the enclosing object that contains the MultiplayerMaze is invalid. 2) Maybe MultiplayerMaze violates the rule of 3, and you're seeing the aftermath of the effects of violating that rule. Since your destructor is not simple, one would think you need a user-defined copy constructor and assignment operator for MazeSceneBuilder. Did you write those functions, and if so, are they buggy or deficient in some way? — PaulMcKenzie, Apr 01 '14 at 22:11
@yoni0505 - All you're showing us are valid usages of the `delete` keyword. That does not tell us the whole story of what is going on. How are these objects used? Are they copied, or used in a container anywhere that makes copies? If so, and again, did you implement the rule of 3 (assignment op, copy ctor, destructor) for those objects that are being copied? How about posting the class definition for MultiplayerMaze and MazeSceneBuilder so that we get a better picture of the setup of these classes? — PaulMcKenzie, Apr 01 '14 at 22:19
Also, you say that MazeSceneBuilder is a member of MultiplayerMaze. That is false, according to the code you posted. You have a *pointer* to a MazeSceneBuilder, not a MazeSceneBuilder object. If you had a MazeSceneBuilder object, then the destructor would be called "naturally" without your intervention. Since the member is really a pointer, then things change drastically. It's you that has to manage the lifetime properly, and not the compiler. — PaulMcKenzie, Apr 01 '14 at 22:25
@PaulMcKenzie I posted the definitions. I don't use copy constructors with these classes. — yoni0505, Apr 01 '14 at 22:27
@yoni0505 - but you didn't disable copying. So if a copy is made of the MazeSceneBuilder object, your program will exhibit undefined behavior. But seriously, we need to have a running program to see how you're using these classes. There is too much that could go wrong with all of those pointers and members. Maybe reworking your code to use smart pointers instead of naked, raw pointers would help a lot, if not eliminate the issues you have now. — PaulMcKenzie, Apr 01 '14 at 22:30
@PaulMcKenzie Smart pointers is a good idea. I could use one for the GameObjectManager and then I wouldn't have to worry about deleting GameObjects. Let's hope it will work, thanks. — yoni0505, Apr 01 '14 at 22:36
I would start with the pointers you're currently dynamically allocating memory for. GameObjects may be one, but all of those others you have "delete" for in the destructor -- I think those are candidates also. Just remember to pick the right smart pointer, unique_ptr, shared_ptr, etc. — PaulMcKenzie, Apr 01 '14 at 22:42

score 1 · Answer 1 · answered Apr 01 '14 at 23:05

1

This is probably not related to the destructor, but please have a look at:

// Remove walls
for (unsigned int x = 0; x < mazeWidth; x++)
{
    for (unsigned int z = 0; z < mazeWidth; z++)
    {
        if (wallGameObjects[x][z] != nullptr)
        {
            mGameObjectManager->removeGameObject(wallGameObjects[x][z]);
        }
    }
}

You use mazeWidth twice. Shouldn't you use mazeHeight? If mazes are not square, you may end up using some uninitialized pointer in wallGameObjects, with unforeseeable side effects, especially if those objects have a destructor that destroys other objects, and so on.

As the classes are complex, I suggest using std::unique_ptr<T> when possible to model ownership. It will help you tracking the bug.

answered Apr 01 '14 at 23:05

ragazzojp

477
3
14

1

Thanks for pointing that out, but still the problem is that all the member variables (including mazeWidth & mazeHeight) are already released. So this bug is unrelated. – yoni0505 Apr 01 '14 at 23:55
The only way to have the destructor of `MazeSceneBuilder` called with a null `this` is skipping the check for null that `delete` does. Now I'm speculating: this might be possible if the `MazeSceneBuilder` is on the stack or is a member of another class, like `MultiplayerMaze`. What's its lifecycle? How do you delete a `MultiplayerMaze`? What's inside `BaseApplication`? – ragazzojp Apr 02 '14 at 00:28

C++ - Object is destroyed before the destructor is executed

1 Answers1