Search code examples
c++inheritancememory-managementshared-ptrvirtual-destructor

c++ shared_ptr inheritance memory problems "Bad deallocation"


I tried solving a university problem by following the given instructions. The code compiles but it produces a warning about memory. Here is the crux of the code:

#include <iostream>
#include <vector>
#include <memory>    
using std::vector;
using std::shared_ptr;
using std::make_shared;

class ApsClass {
    protected :
        double goose;

    public :
        virtual ~ApsClass() {}
        virtual shared_ptr<ApsClass> clone() const = 0;
        double GetGoose() const { return goose; }
};


class DerivedClass : public ApsClass {
    public :
        ~DerivedClass() {} // Warning here*
        DerivedClass(double goose) { DerivedClass::goose = goose; }
        shared_ptr<ApsClass> clone() const override;
};

shared_ptr<ApsClass> DerivedClass::clone() const {
    return make_shared<DerivedClass>(goose);
}

class Storage {
    vector<shared_ptr<ApsClass>> geese;

    public :
        Storage() {}
        ApsClass *AddApsClass(ApsClass *pok);
        void DelApsClass(ApsClass *pok);
};

ApsClass *Storage::AddApsClass(ApsClass *pok)
{
    // The instructions stated that the object pointed at by pok has no
    // other pointers pointing at it, and should now be "owned" by geese
    geese.push_back(shared_ptr<ApsClass>(pok));
    return pok;
}

void Storage::DelApsClass(ApsClass *pok)
{
    for (int i = 0; i < geese.size(); i++)
        if (geese[i] == shared_ptr<ApsClass>(pok)) {
            geese.erase(geese.begin() + i);
            break;
        }
}

int main ()
{
    Storage Foo;
    ApsClass *s = Foo.AddApsClass(new DerivedClass(0.5));
    Foo.DelApsClass(s);
    return 0;
}

main() is basically the code from an autotest, I am supposed to modify the rest of the program to make main() work properly.
The warning is reported at the destructor definition of the derived class*, it just says "Bad deallocation" when translated to English. My question is: Where and why does this problem occur in the code?

The code I've cut out has no effect on the memory allocation (I hope), and is tested via autotesting which i don't have much access to. One test reports the following among the others:

Invalid read of size 8 ...  
Address 0x51f0090 is 0 bytes inside a block of size 56 free'd 

and

==10216== Jump to the invalid address stated on the next line  
==10216== at 0x0: ???  
 Address 0x0 is not stack'd, malloc'd or (recently) free'd

and

==10216== Process terminating with default action of signal 11 (SIGSEGV)
==10216== Bad permissions for mapped region at address 0x0
==10216== at 0x0: ???

If further details are needed I can put some of the original code here.


Solution

  • Every time you do shared_ptr<ApsClass>(pok) you are creating a new shared_ptr that doesn't know about all the other shared_ptrs pointing to the same object. So when this shared_ptr gets destroyed it will destroy the object.

    But because you do it several times (twice, in your case) the object gets destroyed more than once.

    Only shared_ptrs created from normal pointers have this problem. Creating a shared_ptr from another shared_ptr is fine.

    You should create a shared_ptr once - change this

    ApsClass *s = Foo.AddApsClass(new DerivedClass(0.5));
    Foo.DelApsClass(s);
    

    to this:

    shared_ptr<ApsClass> aps(new DerivedClass(0.5));
    shared_ptr<ApsClass> s = Foo.AddApsClass(aps);
    Foo.DelApsClass(s);
    

    and change ApsClass * to shared_ptr<ApsClass> everywhere else.