[C++ SDK] Double free failure with ON_ClassArray


(Menno Deij - van Rijswijk) #1

It took me some time to figure out what was going on, but I think I have run into a double free error with the code below.

ON_NurbsSurface* m_pSurf; // is a member variable
ON_Plane p; // defined elsewhere
ON_ClassArray<ON_SSX_EVENT> srfXPlane;
int n = m_pSurf->IntersectPlane(p.plane_equation, srfXPlane);
	
for (int i = 0; i < n; ++i)
{
	ON_SSX_EVENT ev = srfXPlane[i]; 
	if (ev->IsCurveEvent())
	{
		ON_Curve* crv = ev->m_curve3d;
		// do stuff with curve
	}		
}			
// when this goes out of scope I get an exception.

Then, on a hunch, I modified it to:

ON_ClassArray<ON_SSX_EVENT> srfXPlane;
int n = m_pSurf->IntersectPlane(p.plane_equation, srfXPlane);
	
for (int i = 0; i < n; ++i)
{
	ON_SSX_EVENT* ev = srfXPlane.At(i); // don't use [i] indexing operator, this will incur a double free error
	if (ev->IsCurveEvent())
	{
		ON_Curve* crv = ev->m_curve3d;
                // do stuff with curve
	}		
}		

Using the At(index) function (which returns a pointer instead of a copy) the exception went away.

Can you confirm that this is the case?
What is the difference between ON_ClassArray and ON_SimpleArray?


(Andrew le Bihan) #2

The problem is that ON_SSX_EVENT does not properly implement copying. You can see this from the class definition. There is no operator=, no copy constructor - and there are pointer members that are deleted in the destructor.

So the problem is not with ON_ClassArray - it is that you are copying the ON_SSX_EVENT object.

The correct code would replace this line:

ON_SSX_EVENT ev = srfXPlane[i];

with this:

const ON_SSX_EVENT& ev = srfXPlane[i];


(Andrew le Bihan) #3

The difference between ON_SimpleArray and ON_ClassArray is noted in the header:

////////////////////////////////////////////////////////////////
//
// The ON_ClassArray<> template is designed to be used with
// classes that require non-trivial construction or destruction.
// Any class used with the ON_ClassArray<> template must have a
// robust operator=().
//
// By default, ON_ClassArray<> uses onrealloc() to manage
// the dynamic array memory. If you want to use something
// besides onrealloc() to manage the array memory, then override
// ON_ClassArray::Realloc(). In practice this means that if your
// class has members with back-pointers, then you cannot use
// it in the defaule ON_ClassArray. See ON_ObjectArray
// for an example.


(Menno Deij - van Rijswijk) #4

Ah, thanks for explaining this. My C++ skills are a bit rusty :smile:


(Steve Baer) #5

I added a bugtrack issue for this since we shouldn’t allow this to happen
http://mcneel.myjetbrains.com/youtrack/issue/RH-25052