From b1e03c2292df25b85d4b8986ca635ff26c0e6003 Mon Sep 17 00:00:00 2001
From: wandererfan <wandererfan@gmail.com>
Date: Mon, 17 Jul 2023 14:23:22 -0400
Subject: [PATCH] [TD]fix #9265 - memory leak in cosmetic features

---
 src/Mod/TechDraw/App/CosmeticExtension.cpp    | 31 ++++++++++++++++---
 .../TechDraw/App/PropertyCenterLineList.cpp   |  3 ++
 .../TechDraw/App/PropertyCosmeticEdgeList.cpp | 10 +++---
 3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/src/Mod/TechDraw/App/CosmeticExtension.cpp b/src/Mod/TechDraw/App/CosmeticExtension.cpp
index bfb13881c9..183d5761d4 100644
--- a/src/Mod/TechDraw/App/CosmeticExtension.cpp
+++ b/src/Mod/TechDraw/App/CosmeticExtension.cpp
@@ -43,7 +43,7 @@ CosmeticExtension::CosmeticExtension()
 
     EXTENSION_ADD_PROPERTY_TYPE(CosmeticVertexes, (nullptr), cgroup, App::Prop_Output, "CosmeticVertex Save/Restore");
     EXTENSION_ADD_PROPERTY_TYPE(CosmeticEdges, (nullptr), cgroup, App::Prop_Output, "CosmeticEdge Save/Restore");
-    EXTENSION_ADD_PROPERTY_TYPE(CenterLines ,(nullptr), cgroup, App::Prop_Output, "Geometry format Save/Restore");
+    EXTENSION_ADD_PROPERTY_TYPE(CenterLines ,(nullptr), cgroup, App::Prop_Output, "CenterLine Save/Restore");
     EXTENSION_ADD_PROPERTY_TYPE(GeomFormats ,(nullptr), cgroup, App::Prop_Output, "Geometry format Save/Restore");
 
     initExtensionType(CosmeticExtension::getExtensionClassTypeId());
@@ -51,6 +51,8 @@ CosmeticExtension::CosmeticExtension()
 
 CosmeticExtension::~CosmeticExtension()
 {
+    // do not free memory here as the destruction of the properties will
+    // delete any entries.
 }
 
 TechDraw::DrawViewPart*  CosmeticExtension::getOwner()
@@ -65,8 +67,13 @@ TechDraw::DrawViewPart*  CosmeticExtension::getOwner()
 //if you are creating a CV based on calculations of mirrored geometry, you need to
 //mirror again before creation.
 
+// this is never called!
 void CosmeticExtension::clearCosmeticVertexes()
 {
+    std::vector<TechDraw::CosmeticVertex*> cVerts = CosmeticVertexes.getValues();
+    for (auto& vert : cVerts) {
+        delete vert;
+    }
     std::vector<CosmeticVertex*> noVerts;
     CosmeticVertexes.setValues(noVerts);
 }
@@ -207,7 +214,9 @@ void CosmeticExtension::removeCosmeticVertex(std::string delTag)
     std::vector<CosmeticVertex*> cVerts = CosmeticVertexes.getValues();
     std::vector<CosmeticVertex*> newVerts;
     for (auto& cv: cVerts) {
-        if (cv->getTagAsString() != delTag)  {
+        if (cv->getTagAsString() == delTag)  {
+            delete cv;
+        } else {
             newVerts.push_back(cv);
         }
     }
@@ -223,8 +232,13 @@ void CosmeticExtension::removeCosmeticVertex(std::vector<std::string> delTags)
 
 //********** Cosmetic Edge *****************************************************
 
+// this is never called!
 void CosmeticExtension::clearCosmeticEdges()
 {
+    std::vector<TechDraw::CosmeticEdge*> cEdges = CosmeticEdges.getValues();
+    for (auto& edge : cEdges) {
+        delete edge;
+    }
     std::vector<CosmeticEdge*> noEdges;
     CosmeticEdges.setValues(noEdges);
 }
@@ -347,7 +361,9 @@ void CosmeticExtension::removeCosmeticEdge(std::string delTag)
     std::vector<CosmeticEdge*> cEdges = CosmeticEdges.getValues();
     std::vector<CosmeticEdge*> newEdges;
     for (auto& ce: cEdges) {
-        if (ce->getTagAsString() != delTag)  {
+        if (ce->getTagAsString() == delTag)  {
+            delete ce;
+        } else {
             newEdges.push_back(ce);
         }
     }
@@ -363,8 +379,13 @@ void CosmeticExtension::removeCosmeticEdge(std::vector<std::string> delTags)
 
 //********** Center Line *******************************************************
 
+// this is never called!
 void CosmeticExtension::clearCenterLines()
 {
+    std::vector<TechDraw::CenterLine*> cLines = CenterLines.getValues();
+    for (auto& line : cLines) {
+        delete line;
+    }
     std::vector<CenterLine*> noLines;
     CenterLines.setValues(noLines);
 }
@@ -497,7 +518,9 @@ void CosmeticExtension::removeCenterLine(std::string delTag)
     std::vector<CenterLine*> cLines = CenterLines.getValues();
     std::vector<CenterLine*> newLines;
     for (auto& cl: cLines) {
-        if (cl->getTagAsString() != delTag)  {
+        if (cl->getTagAsString() == delTag)  {
+            delete cl;
+        } else {
             newLines.push_back(cl);
         }
     }
diff --git a/src/Mod/TechDraw/App/PropertyCenterLineList.cpp b/src/Mod/TechDraw/App/PropertyCenterLineList.cpp
index 7d635dfe68..6607bb5d9c 100644
--- a/src/Mod/TechDraw/App/PropertyCenterLineList.cpp
+++ b/src/Mod/TechDraw/App/PropertyCenterLineList.cpp
@@ -78,6 +78,9 @@ void PropertyCenterLineList::setValue(CenterLine* lValue)
 void PropertyCenterLineList::setValues(const std::vector<CenterLine*>& lValue)
 {
     aboutToSetValue();
+    for (auto& value : _lValueList) {
+        delete value;
+    }
     _lValueList.resize(lValue.size());
     for (unsigned int i = 0; i < lValue.size(); i++)
         _lValueList[i] = lValue[i];
diff --git a/src/Mod/TechDraw/App/PropertyCosmeticEdgeList.cpp b/src/Mod/TechDraw/App/PropertyCosmeticEdgeList.cpp
index af3af4b7a5..46b60a76cd 100644
--- a/src/Mod/TechDraw/App/PropertyCosmeticEdgeList.cpp
+++ b/src/Mod/TechDraw/App/PropertyCosmeticEdgeList.cpp
@@ -55,8 +55,6 @@ PropertyCosmeticEdgeList::~PropertyCosmeticEdgeList()
 
 void PropertyCosmeticEdgeList::setSize(int newSize)
 {
-//    for (unsigned int i = newSize; i < _lValueList.size(); i++)
-//        delete _lValueList[i];
     _lValueList.resize(newSize);
 }
 
@@ -69,6 +67,7 @@ int PropertyCosmeticEdgeList::getSize() const
 //_lValueList is not const. so why do we pass a const parameter?
 void PropertyCosmeticEdgeList::setValue(CosmeticEdge* lValue)
 {
+//    Base::Console().Message("PCEL::setValue() - current values: %d lValue: %s\n", _lValueList.size(), lValue ? "valid" : "null");
     if (lValue) {
         aboutToSetValue();
         _lValueList.resize(1);
@@ -79,10 +78,13 @@ void PropertyCosmeticEdgeList::setValue(CosmeticEdge* lValue)
 
 void PropertyCosmeticEdgeList::setValues(const std::vector<CosmeticEdge*>& lValue)
 {
+//    Base::Console().Message("PCEL::seValues() - in values: %d current values: %d\n", lValue.size(), _lValueList.size());
     aboutToSetValue();
     _lValueList.resize(lValue.size());
-    for (unsigned int i = 0; i < lValue.size(); i++)
-        _lValueList[i] = lValue[i];
+    if (!lValue.empty()) {
+        for (unsigned int i = 0; i < lValue.size(); i++)
+            _lValueList[i] = lValue[i];
+    }
     hasSetValue();
 }