When Are Destructors Called?

Question

I'm trying to create a function to copy a class that has pointer members, so the pointers are pointing at a copy rather than the original. There is one particular instance where I don't want changes to carry over to the original.

The problem comes when the destructor is called before the BoardState object should be deleted. So when the Copy function is called again, it is trying to delete the pointers and it can't because they were already deleted.

The strange thing is that if I remove the destructor, everything works. So I would think that if the object was destroyed, but the pointers weren't deleted then the allocated memory would be severed from the pointers and cause a memory leak. However, that is not what happens. The pointers still keep their values. So to me it seems that the destuctor is getting called without deleting the object.

I know I can use smart pointers and not worry about using a destructor, but I want to get a learning experience out of this. So I was hoping someone would be able to tell me what is happening.

Copy Function:

void BoardState::Copy(BoardState state)
{
    copy = true;

    if (p1 != nullptr) {
        delete p1;
    }
    p1 = new Pawn(state.getP1());

    if (p2 != nullptr) {
        delete p2;
    }
    p2 = new Pawn(state.getP2());

    if (walls != nullptr) {
        delete walls;
    }
    walls = new list<Wall>(state.getWalls());
}

Destructor:

BoardState::~BoardState()
{
    if (copy) {
        if (p1 != nullptr) {
            delete p1;
        }
        if (p2 != nullptr) {
            delete p2;
        }
        if (walls != nullptr) {
            delete walls;
        }
    }
}

At the end of this function, the destructor for SimulatedBoard is called:

bool AI::startThinking(Pawn& pawn, Board& board)
{
    simulatedBoard.Copy(board.getBoardState());
    allPossibleDecision.clear();
    plan.clear();
    WallOptions.clear();

    if (!thinking) {
        thinking = true;
    }

    PathFinder pf(simulatedBoard);
    list<sf::Vector2f> path = pf.createPath(board.getPawn(m_turnPos)->getPawn().getPosition(), sf::Vector2f(0, m_goal));

    int opponent_turnPos = 1;
    if (m_turnPos == 1) {
        opponent_turnPos = 2;
    }

    int opponent_goal = 160;
    if (m_goal == 160) {
        opponent_goal = 640;
    }
    list<sf::Vector2f> opponent_path = pf.createPath(board.getPawn(opponent_turnPos)->getPawn().getPosition(), sf::Vector2f(0, opponent_goal));

    int difference = opponent_path.size() - path.size();

    int i;
    if (difference < 0 && totalWalls > 0) {
        i = 1;
    }
    else {
        i = 2;
    }

    switch (i) {
    case 1: {
        list<decision>::iterator nextMove;
        Wall placeWall;
        bool foundBetterDifference = false;

        addWallOptions(sf::Vector2f(190, 190),
            simulatedBoard.getPawn(m_turnPos).getPawn().getPosition(),
            simulatedBoard.getPawn(opponent_turnPos).getPawn().getPosition());

        for (list<decision>::iterator it = allPossibleDecision.begin(); it != allPossibleDecision.end(); it++) {
            decision d = (*it);
            Wall w(d.wallPlacement, wallColor);
            if (d.rotateWall) {
                w.Rotate();
            }

            simulatedBoard.addWall(w);
            opponent_path = pf.createPath(board.getPawn(opponent_turnPos)->getPawn().getPosition(), sf::Vector2f(0, opponent_goal));
            path = pf.createPath(board.getPawn(m_turnPos)->getPawn().getPosition(), sf::Vector2f(0, m_goal));
            simulatedBoard.removeWall(w);

            int newDifference = opponent_path.size() - path.size();
            if (newDifference > difference) {
                foundBetterDifference = true;
                difference = newDifference;
                nextMove = it;
                placeWall = w;
            }
        }

        if (foundBetterDifference) {
            board.addWall(placeWall);
            plan.push_back(*nextMove);
            totalWalls--;
            break;
        }
    }
    case 2 : 
        decision d;
        d.movePawn = true;
        d.pawnPos = path.front();
        plan.push_back(d);
        board.getPawn(m_turnPos)->getPawn().setPosition(path.front());
    }

    return false;
}

SimulatedBoard is not created within this function. It is a member of the class. Even if the class goes out of scope and that is what is deleting the SimulatedBoard, the next time the class is in scope again the constructor should run for SimulatedBoard and set the pointers back to nullptr. Unless my understanding is wrong, which it very well could be.

This doesn’t address the question, but you don’t need to test for nullptr before deleting. delete works just fine with null pointers. — Pete Becker
Write a real copy constructor and assignment operator for BoardState instead of a Copy function. More than likely, you are passing or returning BoardState by value somewhere, or making copies of BoardState. Your BoardState lacks a user-defined copy constructor and assignment operator. Read up on the rule of 3 — PaulMcKenzie
As to your question "when are destructors called". Either when your object goes out of scope, or if dynamically created with new / new[], a corresponding call to delete / delete[] is executed. — PaulMcKenzie
And also, since the code is using C++11 or later, you should have user-defined move constructor and move assignment operator, too. Rule of 5. — Remy Lebeau
Yes, the rule of 5. To the OP, if you implemented the functions mentioned, and implement them without bugs, your problems with the destructor being called may just magically disappear. — PaulMcKenzie

Danilа Maslennikov Danilа Maslennikov · Accepted Answer · 2018-11-03T22:17:42

I would recommend you to define a proper copy constructor for your BoardState class instead of using Copy function.

I suppose that on the line PathFinder pf(simulatedBoard) simulatedBoard is passed by value to the constructor of PathFinder. The result of pf(simulatedBoard) will use the destructor of the simulatedBoard copy.

Since simulatedBoard has copy = true, its copy will have this flag as well, so delete will be called for p1, p2 and walls pointers. Note that the same is happening in the Copy function(the destructor will be called from state argument). Until you define a copy constructor for BoardState you should not pass it by value, because the resulting copy will have a `copy = true' flag.

When Are Destructors Called?

1 Answers