Copy constructor causes memory leak

Question

I've checked this post, and tried the suggestions mentioned, but instead of getting memory leaks I get segmentation faults.

This is the code that causes the leak:

class RecordWithKey : public VarLengthRecord {

    protected:
        Key* Key;
};

class Key {

    protected:
        KeyType keyType;
        Key* Keys[2];

};

RecordWithKey& RecordWithKey::operator=(const RecordWithKey& other)
{
    if (this != &other)
    {
        this->Key = new Key(*(other.Key));
        /* ... other code ... */
    }
    return *this;
}

RecordWithKey::~RecordWithKey()
{
    delete(Key);
    Key = NULL;
}

Key::~Key()
{
    for (unsigned int i = 0; i < KEY_TYPES; i++)
    {
        delete(Keys[i]);
        Keys[i] = NULL;
    }
}

Key& Key::operator =(const Key& other)
{
    if (this != &other)
    {
        this->keyType= other.keyType;

        for(unsigned int i=0;i<KEY_TYPES;i++)
            (*Keys[i]) = (*other.Keys[i]);
    }
    return (*this);
}


  RecordWithKey::RecordWithKey()
    :VarLengthRecord()
{
    Key = NULL;
}

RecordWithKey::RecordWithKey(const RecordWithKey& other)
    : VarLengthRecord(other)
{
    Key = new Key(*(other.Key));
}

Key::Key()
{
    this->keyType = TYPE_STRING; //default type
    this->Keys[TYPE_NUMERIC]= new NumericKey;
    this->Keys[TYPE_STRING]= new StringKey;
}


Key::Key(const Key& other)
{
    this->keyType = other.keyType;
    this->Keys[TYPE_NUMERIC]= new NumericKey;
    this->Keys[TYPE_STRING]= new StringKey;

    if (this->keyType == TYPE_NUMERIC)
    {
        int Key;
        other.get_Key(Key);
        set_Key(Key); //there is no "new" inside of this
    }
    else if (this->keyType == TYPE_STRING)
    {
        std::string Key;
        other.get_Key(Key);
        set_Key(Key); //there is no "new" inside of this
    }
}

This is the report from valgrind:

==15019== 8 bytes in 1 blocks are indirectly lost in loss record 1 of 41
==15019==    at 0x402B9B4: operator new(unsigned int) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==15019==    by 0x80673E6: Key::Key(Key const&) (Key.cpp:21)
==15019==    by 0x806B414: RecordWithKey::RecordWithKey(RecordWithKey const&) (RecordWithKey.cpp:20)
==15019==    by 0x805B8F6: __gnu_cxx::new_allocator<RecordWithKey>::construct(RecordWithKey*, RecordWithKey const&) (in /project/tests/tests)
==15019==    by 0x805BA55: std::vector<RecordWithKey, std::allocator<RecordWithKey> >::_M_insert_aux(__gnu_cxx::__normal_iterator<RecordWithKey*, std::vector<RecordWithKey, std::allocator<RecordWithKey> > >, RecordWithKey const&) (vector.tcc:335)
==15019==    by 0x805B370: std::vector<RecordWithKey, std::allocator<RecordWithKey> >::push_back(RecordWithKey const&) (stl_vector.h:834)
==15019==    by 0x806FF73: NodoSecuencial::insertar(RecordWithKey const&, std::vector<RecordWithKey, std::allocator<RecordWithKey> >&) (NodoSecuencial.cpp:102)
==15019==    by 0x8059A8E: TestNodoSecuencial::test_nodo_sec_insertar_eliminar() (TestNodoSecuencial.cpp:200)
==15019==    by 0x8058400: TestNodoSecuencial::ejecutar() (TestNodoSecuencial.cpp:22)
==15019==    by 0x8076D91: main (tests.cpp:261)

Any ideas? Thanks!

I don't see a destructor, or assignment operator, (or move versions), or RAII. — chris, May 14 '13 at 19:50
`Key = new Key(*(other.Key));` join `Key = NULL;` in constructor == possible dereferencing nullptr. — Lol4t0, May 14 '13 at 19:51
for every `new` that I see I should see a `delete` somewhere. — rwols, May 14 '13 at 19:55
Can we see the class definitions of `Key` and `RecordWithKey`? — rwols, May 14 '13 at 19:58
Out of interest, why does `RecordWithKey` have a `Key*` member rather than just a `Key` member? That would remove a load of grief. — Roddy, May 14 '13 at 20:36
And - do `NumericKey` and `StringKey` inherit from Key? If so, your constructor will recurse indefinitely... — Roddy, May 14 '13 at 20:38

Mark B · Accepted Answer · 2013-05-14T20:36:30.557

2

Your problem is in the copy assignment operator: this->Key = new Key(*(other.Key)); leaks the previously allocated Key object.

The best C++-idiomatic solution is to just not using newly allocated items. Just store your Key and Keys attributes by value and the problem just goes away.

edited May 14 '13 at 20:36

answered May 14 '13 at 20:27

Mark B

95,107
10
109
188

score 0 · Answer 2 · answered May 14 '13 at 19:52

0

It looks like your Key field is declared as Key *Key;. You should delete Key; in RecordWithKey's destructor then.

answered May 14 '13 at 19:52

nullptr

11,008
1
23
18

Copy constructor causes memory leak

2 Answers2