fix double free and memory leak in SHAPE_POLY_SET

There were two problems in the triangulation caching
of SHAPE_POLY_SET:
First there was a double free:
While SHAPE_POLY_SET implements the copy constructor,
it did not implement the operator=, which resulted
in the default operator= being generated by the
compiler. The default operator= copied the member
m_triangulatedPolys, which is a std::vector of pointers.
So after operator= execution, there are two SHAPE_POLY_SET
having pointers to the same TRIANGULATED_POLYGONs, each
of them deleting them in their destructors. This led
to segfaults, because calling
TransformShapeWithClearanceToPolygon on a Zone
uses operator= to copy the contained SHAPE_POLY_SET.
The new SHAPE_POLY_SET then went out of scope and
deleted the cached triangulation within the Zone.

This first problem is fixed by implementing operator=
for SHAPE_POLY_SET.

Second, there was a memory leak: Calling
"CacheTriangulation" on a SHAPE_POLY_SET,
then changing the polygon and then calling
"CacheTriangulation" again led to
leaking the
triangulations generated in the first call.

This second problem is fixed by holding
the cached triangulations in a unique_ptr.
This commit is contained in:
Andreas Buhr 2017-12-08 12:20:02 +01:00 committed by Maciej Suminski
parent d958ab9460
commit b33bf2eafe
2 changed files with 20 additions and 16 deletions

View File

@ -33,7 +33,6 @@
#include <list> #include <list>
#include <algorithm> #include <algorithm>
#include <unordered_set> #include <unordered_set>
#include <memory>
#include <common.h> #include <common.h>
#include <md5_hash.h> #include <md5_hash.h>
@ -58,14 +57,6 @@ SHAPE_POLY_SET::SHAPE_POLY_SET( const SHAPE_POLY_SET& aOther ) :
{ {
} }
SHAPE_POLY_SET::~SHAPE_POLY_SET()
{
for( auto p : m_triangulatedPolys )
delete p;
}
SHAPE* SHAPE_POLY_SET::Clone() const SHAPE* SHAPE_POLY_SET::Clone() const
{ {
return new SHAPE_POLY_SET( *this ); return new SHAPE_POLY_SET( *this );
@ -1871,6 +1862,19 @@ SHAPE_POLY_SET::POLYGON SHAPE_POLY_SET::chamferFilletPolygon( CORNER_MODE aMode,
} }
SHAPE_POLY_SET &SHAPE_POLY_SET::operator=(const SHAPE_POLY_SET & aOther)
{
static_cast<SHAPE&>(*this) = aOther;
m_polys = aOther.m_polys;
// reset poly cache:
m_hash = MD5_HASH{};
m_triangulationValid = false;
m_triangulatedPolys.clear();
return *this;
}
typedef std::map<p2t::Point*, int> P2T_MAP; typedef std::map<p2t::Point*, int> P2T_MAP;
typedef std::vector<p2t::Point*> P2T_VEC; typedef std::vector<p2t::Point*> P2T_VEC;
@ -2022,9 +2026,8 @@ void SHAPE_POLY_SET::CacheTriangulation()
for( int i = 0; i < tmpSet.OutlineCount(); i++ ) for( int i = 0; i < tmpSet.OutlineCount(); i++ )
{ {
auto p = new TRIANGULATED_POLYGON(); m_triangulatedPolys.push_back( std::make_unique<TRIANGULATED_POLYGON>() );
m_triangulatedPolys.push_back( p ); triangulateSingle( tmpSet.Polygon( i ), *m_triangulatedPolys.back() );
triangulateSingle( tmpSet.Polygon( i ), *p );
} }
m_triangulationValid = true; m_triangulationValid = true;

View File

@ -28,6 +28,7 @@
#include <vector> #include <vector>
#include <cstdio> #include <cstdio>
#include <memory>
#include <geometry/shape.h> #include <geometry/shape.h>
#include <geometry/shape_line_chain.h> #include <geometry/shape_line_chain.h>
@ -406,8 +407,6 @@ class SHAPE_POLY_SET : public SHAPE
*/ */
SHAPE_POLY_SET( const SHAPE_POLY_SET& aOther ); SHAPE_POLY_SET( const SHAPE_POLY_SET& aOther );
~SHAPE_POLY_SET();
/** /**
* Function GetRelativeIndices * Function GetRelativeIndices
* *
@ -585,7 +584,7 @@ class SHAPE_POLY_SET : public SHAPE
const TRIANGULATED_POLYGON* TriangulatedPolygon( int aIndex ) const const TRIANGULATED_POLYGON* TriangulatedPolygon( int aIndex ) const
{ {
return m_triangulatedPolys[aIndex]; return m_triangulatedPolys[aIndex].get();
} }
@ -1120,6 +1119,8 @@ class SHAPE_POLY_SET : public SHAPE
public: public:
SHAPE_POLY_SET& operator=( const SHAPE_POLY_SET& );
void CacheTriangulation(); void CacheTriangulation();
bool IsTriangulationUpToDate() const; bool IsTriangulationUpToDate() const;
@ -1128,7 +1129,7 @@ class SHAPE_POLY_SET : public SHAPE
MD5_HASH checksum() const; MD5_HASH checksum() const;
std::vector<TRIANGULATED_POLYGON*> m_triangulatedPolys; std::vector<std::unique_ptr<TRIANGULATED_POLYGON>> m_triangulatedPolys;
bool m_triangulationValid = false; bool m_triangulationValid = false;
MD5_HASH m_hash; MD5_HASH m_hash;