"pointer being freed was not allocated"

Question

#include <iostream>
#include <fstream>
#include <cmath>
#include <string>
#include <sstream>

using namespace std;

class CFile {
public:
    CFile(string filename); 
    ~CFile();               
    void ReadFile();         
    void WriteFile(string outputFilename);   
    void Calculate();   
    string  m_filename;
    int     m_numberInput;
    double* m_xData;         
    double* m_yData;
    int     m_numberOutput;
    double* m_xDataOut;     
    double* m_yDataOut;
};

CFile::CFile(string filename)
{
    m_filename = filename;
    string line;             
    ifstream myfile(m_filename.c_str());

    if (myfile.is_open())     
    {
        getline(myfile,line);    
        myfile.close();          
        stringstream Str;        
        Str << line; 
        Str >> m_numberInput;
        m_xData = new double[m_numberInput];  
        m_yData = new double[m_numberInput];
        cout << sizeof(m_xData) / sizeof(m_xData[0]) << endl;
    }
    else cout << "Unable to open file.\n";  
}

CFile::~CFile()
{ 
    delete[] m_xData; 
    delete[] m_yData;                                  
    m_xData = 0;                                 
    m_yData = 0;
    delete[] m_xDataOut; 
    delete[] m_yDataOut;                                  
    m_xDataOut = 0;                                 
    m_yDataOut = 0;
}

void CFile::ReadFile()
{
    ifstream infile(m_filename.c_str());
    string line;

    if (infile.is_open())
    {
        int x, y, i = 0;

        while (getline(infile,line))
        {
            infile >> x >> y;
            m_xData[i] = x;
            m_yData[i] = y;
            i++;
        }
        infile.close();
    }
    else cout << "Unable to open file.\n"; 
}

void CFile::WriteFile(string outputFilename)
{
    ofstream outfile(outputFilename.c_str());

    if (outfile.is_open())
    {
        for(int i=0; i < m_numberInput; i++)
            outfile << m_xDataOut[i] << " " << m_yDataOut[i] << endl;

        outfile.close();
    }
    else cout << "Unable to open file.\n"; 
}

void CFile::Calculate()
{
    m_xDataOut = new double[m_numberInput]; 
    m_yDataOut = new double[m_numberInput];

    for(int i=0; i < m_numberInput; i++)
    {
        m_xDataOut[i] = m_xData[i];
        m_yDataOut[i] = sqrt(m_yData[i]);
    }
}


int main()
{
    CFile file("Input.dat");
    file.ReadFile(); 
    file.Calculate(); 
    file.WriteFile("Output.dat");
    file.~CFile();
}

The error message is as follows:

main(11915,0x7fff77d3d310) malloc: *** error for object 0x7f8a99403940: pointer 
being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
Abort trap: 6

This is code which ironically worked well some weeks ago. I change OS and compilers and now it doesn't anymore. I read about this error in other threads, but couldn't really understand how to use the solutions on my code. The code reads in a data file, does sth. with it and then writes the changed values into another file. Where is the allocation error? Thank you very much in advance for your help! :-)

You delete four arrays in your destructor. But you don't always initialize them. What do you expect? Furthermore, you explicitly call the destructor, but this gets called again when exiting the scope. You do not need that destructor call. — juanchopanza, Jan 28 '14 at 11:19
Apart from your error that breaks this program break, you've also violated the rule-of-three. Instead of fiddling with manual heap (de)allocation, you REALLY should be using std::vector<> instead. — sellibitze, Jan 28 '14 at 11:23
`struct Point { double X,Y; }; typedef std::vector PointVector; PointVector calculate(const &PointVector input){ ...};` — Roddy, Jan 28 '14 at 11:33

score 5 · Accepted Answer · answered Jan 28 '14 at 11:20

5

Your Destructor is called twice. You call it explicitly, and it is also called when exiting main(). You should remove your explicit destrcutor call.

answered Jan 28 '14 at 11:20

divinas

1,787
12
12

`file.~CFile();` this is the line the OP should remove, you mean – nodakai Jan 28 '14 at 11:23

score 2 · Answer 2 · edited May 23 '17 at 12:17

Two problems:

You don't necessarily initialise all four pointers, in which case it's not safe to apply delete to them. You can fix this by initialising them to null before doing anything; or, better still, replace them with std::vector<double> so you don't have to mess around with delete at all. That will also fix the class's invalid copy semantics (it breaks the Rule of Three) and the potential memory leaks if construction fails.
You're calling the destructor manually (file.~CFile(), at the end of main). Don't do that: the destructor is called automatically when the program leaves the variable's scope, and it's an error to call it twice.

score 1 · Answer 3 · answered Jan 28 '14 at 11:28

As @claptrap pointed out your memory managements were also buggy.

You should migrate to std::vector<double> rather than raw double arrays and pointers. You can think of vec.resize(N) as a variant of new double[N] which never needs an explicit delete[].
If you are on Linux, it's a good practice to run your program under valgrind which automatically traces memomry allocations/deallocations and points out any invalid memory operations. I bet your Linux distro has a pre-compiled package for it.
- http://valgrind.org/docs/manual/quick-start.html

score 0 · Answer 4 · answered Jan 28 '14 at 11:18

0

This here is causing the issue

  delete[] m_xDataOut; 
  delete[] m_yDataOut;

you have not initialized the pointers in your constructor

answered Jan 28 '14 at 11:18

AndersK

35,813
6
60
86

Although in general you are right, that if you follow RAII you should allocate memory in your constructor and deallocate in the destructor, this is not the OPs problem, because he allocates the memory in his Calcuate() function, which in his example is always called. – divinas Jan 28 '14 at 11:28

"pointer being freed was not allocated"

4 Answers4