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.

Answer 1

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.