Why am I getting munmap_chunk(): invalid pointer: error

Question

So i was doing some assignment and suddenly facing this "invalid pointer" error when I try to "delete" the earlier assigned weaponBehaviour in the "setweapon" function in class Character . Could someone give some pointers as to what might be the problem ?


#include <iostream>
using namespace std;

class WeaponBehaviour{
public:
    virtual void useWeapon() = 0;
};

class SwordBehaviour : public WeaponBehaviour {
public:
    void useWeapon()
    {
        cout << "slash slash"<< endl;
    }
};
class BowAndArrowBehavior : public WeaponBehaviour {
public:
    void useWeapon()
    {
        cout << "suss suss"<< endl;
    }
};

class KnifeBehavior : public WeaponBehaviour {
public:
    void useWeapon()
    {
        cout << "chak chakk"<< endl;
    }
};
class AxeBehavior : public WeaponBehaviour {
public:
    void useWeapon()
    {
        cout << "chop chop"<< endl;
    }
};



class Character{
protected:
    WeaponBehaviour* weaponBehaviour;
public:
    Character(){

    }
    ~Character(){
        if(weaponBehaviour != NULL)
        {
            delete weaponBehaviour;
        }
    }
    void setWeapon(WeaponBehaviour* newWeapon)
    {
        if(weaponBehaviour!= NULL )
            delete weaponBehaviour;

        weaponBehaviour = newWeapon;
    }
    void fight()
    {
        weaponBehaviour->useWeapon();
    }
};

class King : public Character
{
public:
    King(){
        setWeapon(new SwordBehaviour);
    }
};

class Queen : public Character
{
public:
    Queen(){
        setWeapon(new KnifeBehavior);
    }
};

int main() {
    King king ;
    king.fight();
    Queen queen ;
    queen.fight();
    queen.setWeapon(new AxeBehavior);
    queen.fight();
    return 0;
}

the error i am getting is

* Error in `Cpp-design-patters/Debug/strategy-pattern': munmap_chunk(): invalid pointer: 0x0000000000400f50 * ======= Backtrace: ========= /lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7ff746d757e5] /lib/x86_64-linux-gnu/libc.so.6(cfree+0x1a8)[0x7ff746d82698]

You should try using a debugger. It's an invaluable tool for this kind of issue. Also compile with warnings. You have no virtual destructor in WeaponBehaviour, so the deletes in your code are UB. — Paul Rooney
If I read that backtrace correctly (its formatting is a bit messed up), then this is far away from your code. Your code (you should take it to codereview.stackexchange.com, btw, there's lots to improve) doesn't look like it could cause that to me. The only obvious mistake is the lack of virtual destructors. — Ulrich Eckhardt
@PaulRooney thanks for the tip ,the issue was because of not using virtual destructor adding it solved the issue . — hungryspider
ok cool. I added it but still got a segfault. You should pass -Wall -Wextra -pedantic flags to GCC and also -g to include debug symbols. With this GCC warns about the dtor. — Paul Rooney
you never initialize weaponBehaviour to NULL, it has an indeterminate value that is likely not NULL, so you'll try to delete something that you never new'd — kmdreko

Alan Birtles Alan Birtles · Accepted Answer · 2019-10-17T06:59:08

You need to initialise weaponBehaviour to nullptr otherwise weaponBehaviour will have an undefined (possibly non-null) value and

if(weaponBehaviour!= NULL)
  delete weaponBehaviour;

Will pass the if statement and delete an invalid pointer.

You compiler should also warn you (if you have turned on warnings) that you have a virtual class with no virtual destructor. This leads to undefined behaviour when deleting an object via a WeaponBehaviour pointer. The correct declaration of WeaponBehaviour is:

class WeaponBehaviour{
public:
    virtual WeaponBehaviour() {}
    virtual void useWeapon() = 0;
};

It isn't causing a problem in your code (yet) but you should be aware of the rule of three/five.

All of the above would be solved by using std::unique_ptr and would make your code simpler and safer:

#include <iostream>
#include <memory>

class WeaponBehaviour{
public:
    virtual ~WeaponBehaviour(){}
    virtual void useWeapon() = 0;
};

class SwordBehaviour : public WeaponBehaviour {
public:
    void useWeapon()
    {
        std::cout << "slash slash\n";
    }
};
class BowAndArrowBehavior : public WeaponBehaviour {
public:
    void useWeapon()
    {
        std::cout << "suss suss\n";
    }
};

class KnifeBehavior : public WeaponBehaviour {
public:
    void useWeapon()
    {
        std::cout << "chak chakk\n";
    }
};
class AxeBehavior : public WeaponBehaviour {
public:
    void useWeapon()
    {
        std::cout << "chop chop\n";
    }
};



class Character{
protected:
    std::unique_ptr<WeaponBehaviour> weaponBehaviour;
public:
    void setWeapon(std::unique_ptr<WeaponBehaviour>&& newWeapon)
    {
        weaponBehaviour = std::move(newWeapon);
    }
    void fight()
    {
        weaponBehaviour->useWeapon();
    }
};

class King : public Character
{
public:
    King(){
        setWeapon(std::make_unique<SwordBehaviour>());
    }
};

class Queen : public Character
{
public:
    Queen(){
        setWeapon(std::make_unique<KnifeBehavior>());
    }
};

int main() {
    King king ;
    king.fight();
    Queen queen ;
    queen.fight();
    queen.setWeapon(std::make_unique<AxeBehavior>());
    queen.fight();
    return 0;
}

Why am I getting munmap_chunk(): invalid pointer: error

1 Answers