QVector memory managment

Question

I have this very simple dummy program

levenshteindb.h :

#ifndef LEVENSHTEINDB_H
#define LEVENSHTEINDB_H

#include <QVector>
#include "levenshteindbnode.h"

class LevenshteinDB
{
    unsigned size;
    QVector<LevenshteinDBNode> nodes;
    void realloc_rows(unsigned node);

public:
    LevenshteinDB();
    ~LevenshteinDB();
    void add_word();
};

#endif // LEVENSHTEINDB_H

levenshteindb.cpp :

#include "levenshteindb.h"
#include <cstring>
#include <cstdio>    

LevenshteinDB::LevenshteinDB()
{
    size=15;
    nodes.append(LevenshteinDBNode(size));
}

LevenshteinDB::~LevenshteinDB()
{
}


void LevenshteinDB::add_word()
{
    nodes.append(LevenshteinDBNode(size));
}


void LevenshteinDB::realloc_rows(unsigned newsize)
{
    for(unsigned i=0;i<nodes.size();i++)
        nodes[i].realloc(newsize);
}

levenshteindbnode.h :

#ifndef LEVENSHTEINDBNODE_H
#define LEVENSHTEINDBNODE_H

struct LevenshteinDBNode
{
    LevenshteinDBNode();
    LevenshteinDBNode(unsigned size);
    ~LevenshteinDBNode();
    unsigned *row;
    void realloc(unsigned newsize);
};

#endif // LEVENSHTEINDBNODE_H

levenshteindbnode.cpp :

#include "levenshteindbnode.h"

LevenshteinDBNode::LevenshteinDBNode(){};

LevenshteinDBNode::LevenshteinDBNode(unsigned size)
{
    row = new unsigned[size];
}

LevenshteinDBNode::~LevenshteinDBNode()
{
    delete[] row;
}

void LevenshteinDBNode::realloc(unsigned newsize)
{
    delete[] row;
    row=new unsigned[newsize];
}

main.cpp :

#include "levenshteindb.h"

int main()
{
    LevenshteinDB *trie = new LevenshteinDB();
    trie->add_word();
    trie->add_word();
    trie->add_word();
    delete trie;
}

that crashes and seems to have a huge (compared to the memory allocated by the program itself) memory leak, but I really cannot understand what's wrong.. I'm using qt 5.2

score 3 · Accepted Answer · answered Jan 08 '14 at 09:53

3

You need to read about the rule of three.

What's happening is that when you do

nodes.append(LevenshteinDBNode(size));

you create a temporary LevenshteinDBNode objectm which is then copied, leading to two objects with two pointers to the same memory. The temporary object is then destroyed which causes your destructor to be called, and which deletes the memory you've allocated. Now you have a copy of the object, with a pointer to the deleted memory.

You need to implement a copy constructor which does a so called deep copy of the memory allocated.

You also have a much more subtle error in your code, in that you don't initialize the pointer in the LevenshteinDBNode default constructor. This means that if you have a default-constructed instance, the pointer will have an indeterminate value, in reality it will point to a random location. This will lead to undefined behavior if a default-constructed instance is then destructed when you try to delete this random pointer. You need to initialize the pointer to nullptr in the default constructor.

answered Jan 08 '14 at 09:53

Some programmer dude

400,186
35
402
621

thanks for your answer, that surely hit the problem(s), there is just one other thing I'd like to ask, what should I do to fix the problem? I mean, I could define a copy constructor for LevenshteinDBNode that makes a deep copy of the object, but in this way when I call "nodes.append(LevenshteinDBNode(size))" the memory is allocated twice to construct a single object which seems to me not very efficient, so do I need to create two method init() and free() to manually allocate and release "rows" (and erase in constructor and destructor any "new" and "delete")? – woggioni Jan 08 '14 at 10:15
1

@user2318607 Unfortunately that's how it works. If you have a C++11 capable compiler (and Qt is updated to use the new C++11 features) you could use [move semantics](http://stackoverflow.com/a/3109981/440558) to avoid the copying in some situations. But you still need to make a copy-constructor which does a second allocation and a copy of the data. – Some programmer dude Jan 08 '14 at 10:18
4

I think you may also define LevenshteinDBNode::row as QVector. – Nikita Jan 08 '14 at 10:33
@user2318607 you should use existing container implementations which won't have these bugs like `QList` or `std::vector` – ratchet freak Jan 08 '14 at 10:52
Alternatively, you can implement copy-on-write, e.g. using QSharedDataPointer. – Frank Osterfeld Jan 08 '14 at 12:46

QVector memory managment

1 Answers1