Search code examples
c++inheritancedecoratoraccess-violationvirtual-destructor

Access violation - Why is base class destructor getting called twice?


I have a class Player which attempts to implement the Decorator pattern. Playercontains a member of its base class Character called m_player. When calling the destructor of Player from the client, I run into some problems resulting in a memory access violation. Starting in main:

Character* createBaseClass();

// more forward declarations

int main (int argc, char* const argv[]) 
{
    Player* mainCharacter = new Player(createBaseCharacter());

    delete mainCharacter;   // Crashes when calling delete

    return 0;
}

Character* createBaseCharacter()
{
    return Character::Builder()
        .name("Dylan")
        .description("Super bad-ass hero of the game")
        .build();
}

The error occurs shortly after I call the delete operator on mainCharacter, which has the following call sequence:

Player::~Player()
{
    delete m_armor;
    delete m_weapon;
    delete m_player;  // calls Character's destructor
}

Then the destructor for Character:

Character::~Character()
{
    // works fine
    //
    delete m_abilityAttributes;  
    m_abilityAttributes = NULL;
    delete m_primaryAttributes;
    m_primaryAttributes = NULL;
}

However, The strange thing is that this destructor appears to be getting called twice - once the above is done executing the debugger takes me to a disassembly stepping me through a "scalar deleting destructor", which appears to call the Character destructor again, by way of the interface for Player, called CharacterDecorator:

enter image description here

Call stack at point of crash:

enter image description here

Call to CharacterDecorator's destructor results in subsequent call to Character's destructor:

Character::~Character()
{
    // Crashes with Access Violation
    //
    delete m_abilityAttributes;  
    m_abilityAttributes = NULL;
    delete m_primaryAttributes;
    m_primaryAttributes = NULL;
}

At this point I am thoroughly confused - I am not sure why the destructor gets called again through the abstract interface CharacterDecorator in addition to the destructor getting called through its concrete implementation. Additionally, adding a destructor to CharacterDecorator doesn't appear to solve the problem.

For reference, I have included the implementation of Player, Character and the interface for CharacterDecorator:

class CharacterDecorator : public Character
{
public:

    virtual Armor* getArmor() const = 0;
    virtual Weapon* getWeapon() const = 0;
};

Player:

Player::Player()
{}

Player::Player(Character* player)
    :m_player(player)
    ,m_weapon(0)
    ,m_armor(0)
{}

Player::Player(Character* player, Weapon* weapon, Armor* armor)
    :m_player(player) 
    ,m_weapon(weapon)
    ,m_armor(armor)
{}

Player::~Player()
{
    delete m_armor;
    delete m_weapon;
}

// getters
Armor* Player::getArmor() const
{
    return m_armor;
}

Weapon* Player::getWeapon() const
{
    return m_weapon;
}

// additional methods ...

Character:

Character::Character()
{}

Character::Character(const Builder& builder)
    :m_name(builder._name)
    ,m_description(builder._description)
    ,m_abilityAttributes(builder._abilityAttributes)
    ,m_primaryAttributes(builder._primaryAttributes)
{}

Character::Character(const Character& rhs)
{
    m_name = rhs.m_name;
    m_description = rhs.m_description;
    m_abilityAttributes = new AbilityAttributes();
    m_primaryAttributes = new PrimaryAttributes();
    *m_abilityAttributes = *rhs.m_abilityAttributes;
    *m_primaryAttributes = *rhs.m_primaryAttributes;
}

Character::~Character()
{
    delete m_abilityAttributes;
    m_abilityAttributes = NULL;
    delete m_primaryAttributes;
    m_primaryAttributes = NULL;
}

// additional methods ...

// Builder pattern methods
//
Character::Builder::Builder()
    : _abilityAttributes(0), _primaryAttributes(0)
{}

Character* Character::Builder::build()
{
    return new Character(*this);
}

Character::Builder& Character::Builder::abilityAttributes(AbilityAttributes* value)
{
    _abilityAttributes = value;
    return *this;
}

Character::Builder& Character::Builder::primaryAttributes(PrimaryAttributes* value)
{
    _primaryAttributes = value;
    return *this;
}

Solution

  • Though I provided a Constructor that takes no arguments, I didn't give it an initialization list for the members of pointer type:

    Character::Character() 
    {}
    

    Should be:

    Character::Character() 
        : m_abilityAttributes(0), m_primaryAttributes(0)
    {}
    

    I guess this is because the constructor for the base class Character gets called when I instantiate an object of Player, but the default constructor for the base class didn't have an member initialization list before, hence the access violation error when calling the base destructor (?)