Memory Allocation Issue

[aerowenn]

Been a very long day, and rather than post a ton of code, I think I've traced down where the error might be, but I'm unsure what's causing it though. Here is the error:

project043(5827) malloc: *** error for object 0x1001001b0: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug

Suspected class causing issue:

Header triangle.h:

#ifndef TRIANGLE_H
#define TRIANGLE_H

#include "polygon.h"
#include "point.h"

class Triangle : public Polygon {
public:
  Triangle(const Point& p0, const Point& p1, const Point& p2);
  ~Triangle();
  virtual int numberOfPoints() const { return 3; }
  virtual Point point(const int& index) const;
private:
  Point* m_points;
}; // class Triangle

#endif 

Source triangle.cpp:

#include <cassert>

Triangle::Triangle(const Point& p0, const Point& p1, const Point& p2) {
  m_points = new Point[3];
  m_points[0] = p0;
  m_points[1] = p1;
  m_points[2] = p2;
}

Triangle::~Triangle() {
    delete m_points;
}

Point Triangle::point(const int &index) const {
  assert(index>=0);
  assert(index<3);
  return m_points[index];
}

Parent header polygon.h:

#ifndef POLYGON_H
#define POLYGON_H

#include "point.h"

class Polygon {
public:
  virtual int numberOfPoints() const = 0;
  virtual Point point(const int& index) const = 0;
  virtual double circumference() const;
}; // class Polygon

#endif 

Parent source polygon.cpp:

#include "polygon.h"

#include <cassert>

double Polygon::circumference() const {
  assert(numberOfPoints()>2);
  double circumference = 0;
  for(int i=1; i<numberOfPoints(); ++i)
    circumference += point(i-1).distanceTo(point(i));
  assert(circumference>0.0);
  return circumference;
}

Point class header point.h:

#ifndef POINT_H
#define POINT_H

class Point {
public:
  Point(const double& x=0.0, const double& y=0.0);
  double distanceTo(const Point& point) const;
  double distanceToOrigin() const;
  double x() const {return m_x;}
  double y() const {return m_y;}
private:
  double m_x;
  double m_y;
}; // class Point

#endif 

Point class source point.cpp:

#include "point.h"

#include <cmath>

Point::Point(const double& x, const double& y) : m_x(x), m_y(y) {}

double
Point::distanceTo(const Point &point) const {
  const double distance = 
    std::sqrt((m_x-point.m_x)*(m_x-point.m_x)+(m_y-point.m_y)*(m_y-point.m_y));
  return distance;
}

Points of interest in the main function:

Where vector is declared:

...
  srand(static_cast<unsigned int>(time(NULL)));
  
  std::vector<Polygon*> polygons;
  std::vector<Triangle> triangles;
  
  // preallocate the vector
  polygons.reserve(SIZE);
  triangles.reserve(SIZE);
    
  int triangle_count  = 0;
  int square_count    = 0;
  int rectangle_count = 0;
...

Where vector is filled:

...
 for(unsigned i=0; i<SIZE; ++i){
    int type = rand()%NUM_OF_TYPES;
    switch (type) {
      case 0:
      {
        const Point p0(static_cast<double>(rand()),
                       static_cast<double>(rand()));
        const Point p1(static_cast<double>(rand()),
                       static_cast<double>(rand()));
        const Point p2(static_cast<double>(rand()),
                       static_cast<double>(rand()));
        polygons.push_back(new Triangle(p0, p1, p2));
        triangles.push_back(Triangle(p0, p1, p2));
        ++triangle_count;
      }
...

Side length calculation:

// compute triangles only
  double triangle_length = 0.0;
  for(unsigned i=0; i<triangles.size(); ++i)
    triangle_length += triangles[i].circumference();

I know I said not a lot of code, so I tried to not post anything I didn't find relevant. I'm not looking for someone just to fix it for me, rather just help point out what could be wrong so I can understand the mistake.

Thanks!

[ciphermagi]

This may be a knee-jerk reaction, but my first instinct says that

#include <cassert>

Triangle::Triangle(const Point& p0, const Point& p1, const Point& p2) {
  m_points = new Point[3];
  m_points[0] = p0;
  m_points[1] = p1;
  m_points[2] = p2;
}

Triangle::~Triangle() {
    delete m_points; // This line is causing the error (according to instinct)
}

Point Triangle::point(const int &index) const {
  assert(index>=0);
  assert(index<3);
  return m_points[index];
}

in your destructor above, you are deleting a variable that's out of scope.

[aerowenn]

Great catch! Commenting that line actually gets rid of the error, however, are we not looking at a memory leak if m_points is not deleted in some way?

[aerowenn]

Also, thanks! Forgot my manors.

[ciphermagi]

Actually, yes it is a potential memory leak if it's not properly handled.

[aerowenn]

Well, I can't figure this one out, the destructor is called when it goes out of scope, but if it's gone out of scope it has issues as well. Should the deletion be done in the destructor?

[ciphermagi]

No, actually. You can't delete a variable that's already been de-allocated, but without having deleted it, it's still holding that memory for you until you terminate the program. So you need to either get rid of it, or have some way to access the information (i.e. pointers) in place before the constructor finishes.

[iperetta]

In your code:

  m_points = new Point[3];
  ...
    delete m_points;

m_points is an array, it should be deleted by:

    delete [] m_points;

Create a method like "void deletePoints() { delete [] this->m_points; }" and call it within main() before the program ends, or at least after the last use of Triangle you should have.

[helios]

The fact that the program crashed on that particular line doesn't say much, other than strongly suggest that there's a memory access problem somewhere in the program that's probably doing something like

((unsigned *)m_points)[-1]=garbage;

The program probably crashed then because free() happened to need that piece of information. The beauty is that the actual problem can literally be anywhere in the program.

Are you on Linux? Can you run the program through Valgrind?

[aerowenn]

@ciphermagi
Thanks, that makes sense. I'm filling a vector in main with new Triangles and just Triangles, each with about 300K objects, it would be nice if it could be accomplished within the class, but I don't see it.

@iperetta
I will give something like this a shot, thanks!

@helios
Something I've noticed about memory leaks and other problems. Removing that portion of code resulted in no allocation errors, but I'm almost positive there is still a memory leak.

I am currently on a Mac, but I also have a Linux desktop that I use for development, as well as a Windows machine. I'll check out Valgrind, thanks!