Double-free error in execution after deleting pointer in deconstructor

Question

I have a class containing a member pointer which is dynamically allocated in its constructor as follows:

class Record {
public:
    Record(unsigned short numBytes, char* bufRecord);
    ~Record();

    unsigned short size() {return m_numBytes;}
private:
    unsigned short m_numBytes;
    char* m_bufRecord;
};

Record::Record(unsigned short numBytes, char* bufRecord) {
    m_numBytes = numBytes;
    m_bufRecord = new char[numBytes];

    for(unsigned short i=0; i<numBytes; i++)
        m_bufRecord[i] = bufRecord[i];
}

Record::~Record() {
    delete m_bufRecord;
}

It basically copies the input buffer into the dynamically allocated member buffer. I proceed to use this class as follows, in the constructor of another class:

class File {
public:
    File(const char* fileName);
    ~File();

    unsigned int numRecords() {return m_records.size();}
    Record getRecord(unsigned int numRecord) {return m_gdsRecords[numRecord];}
private:
    std::ifstream           m_file;
    std::vector<Record>     m_records;
};

File::File(const char* fileName) : m_file(fileName, ios::in | ios::binary) {
    while(!m_file.eof()) {
        char bufNumBytes[2];
        char* bufRecord;
        unsigned short numBytes;

        m_file.read(bufNumBytes, 2);
        numBytes = (bufNumBytes[0] << 8) + bufNumBytes[1] - 2;
        bufRecord = new char[numBytes];
        m_file.read(bufRecord, numBytes);

        Record record(numBytes, bufRecord);
        m_records.push_back(record);

        delete bufRecord;
    }
}

However, when I instantiate this class, I get the following error, which seems to state that I'm double-freeing the m_bufRecord:

*** Error in `./a.out': double free or corruption (fasttop): 0x0000000001cb3280 ***

I'm guessing the problem lies with the insertion of a class containing a pointer to the vector element, and the destructor being called twice on the same pointer but I'm not sure how this happens. What am I doing wrong here?

Please read up on the [Rule of Three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). — Oliver Charlesworth, Feb 10 '13 at 17:22
@OliCharlesworth About the post the same thing. cgurleyuk, the pointer is being copied into the `std::vector`, so both objects are `delete`ing it. You should either use a `std::string`/`std::vector` for the buffer or a `std::shared_ptr` for the pointer. — Alex Chamberlain, Feb 10 '13 at 17:24
Considering its usage, I'd toss Record entirely and just manage a `std::vector< std::vector >` — WhozCraig, Feb 10 '13 at 17:41

score 1 · Accepted Answer · answered Feb 10 '13 at 17:35

This is a case of the Rule of three. If your class needs to free resources in the destructor, it generally needs to declare a copy constructor (and copy assignment operator), to either copy the owned resource, manage shared ownership or prevent being copied.

score 0 · Answer 2 · answered Feb 10 '13 at 17:26

Record getRecord(unsigned int numRecord) {return m_gdsRecords[numRecord];}

This function returns a copy of the Record. So now you have two Records containing the same m_bufRecord pointer. Running the destructors on these Records will try to delete the same pointer value twice.

Double-free error in execution after deleting pointer in deconstructor

2 Answers2