From 82101ac89049eea6e89a9c57c83ccf5282a3c03c Mon Sep 17 00:00:00 2001 From: wmayer Date: Wed, 8 May 2024 16:53:29 +0200 Subject: [PATCH] Part: Fix memory leaks --- src/Mod/Part/App/PropertyGeometryList.cpp | 26 ++++++++++++--------- src/Mod/Part/App/PropertyTopoShapeList.cpp | 25 ++++++++++---------- src/Mod/Part/parttests/TopoShapeListTest.py | 1 + src/Mod/Part/parttests/TopoShapeTest.py | 2 ++ 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/Mod/Part/App/PropertyGeometryList.cpp b/src/Mod/Part/App/PropertyGeometryList.cpp index fe4252139b..23f9660721 100644 --- a/src/Mod/Part/App/PropertyGeometryList.cpp +++ b/src/Mod/Part/App/PropertyGeometryList.cpp @@ -118,10 +118,11 @@ void PropertyGeometryList::set1Value(int idx, std::unique_ptr &&lValue PyObject *PropertyGeometryList::getPyObject() { - PyObject* list = PyList_New(getSize()); - for (int i = 0; i < getSize(); i++) - PyList_SetItem( list, i, _lValueList[i]->getPyObject()); - return list; + Py::List list; + for (int i = 0; i < getSize(); i++) { + list.append(Py::asObject(_lValueList[i]->getPyObject())); + } + return Py::new_reference_to(list); } void PropertyGeometryList::setPyObject(PyObject *value) @@ -130,30 +131,33 @@ void PropertyGeometryList::setPyObject(PyObject *value) Part2DObject* part2d = dynamic_cast(this->getContainer()); if (PySequence_Check(value)) { - Py_ssize_t nSize = PySequence_Size(value); + Py::Sequence sequence(value); + Py_ssize_t nSize = sequence.size(); std::vector values; values.resize(nSize); for (Py_ssize_t i=0; i < nSize; ++i) { - PyObject* item = PySequence_GetItem(value, i); - if (!PyObject_TypeCheck(item, &(GeometryPy::Type))) { + Py::Object item = sequence.getItem(i); + if (!PyObject_TypeCheck(item.ptr(), &(GeometryPy::Type))) { std::string error = std::string("types in list must be 'Geometry', not "); - error += item->ob_type->tp_name; + error += item.ptr()->ob_type->tp_name; throw Base::TypeError(error); } - values[i] = static_cast(item)->getGeometryPtr(); + values[i] = static_cast(item.ptr())->getGeometryPtr(); } setValues(values); - if (part2d) + if (part2d) { part2d->acceptGeometry(); + } } else if (PyObject_TypeCheck(value, &(GeometryPy::Type))) { GeometryPy *pcObject = static_cast(value); setValue(pcObject->getGeometryPtr()); - if (part2d) + if (part2d) { part2d->acceptGeometry(); + } } else { std::string error = std::string("type must be 'Geometry' or list of 'Geometry', not "); diff --git a/src/Mod/Part/App/PropertyTopoShapeList.cpp b/src/Mod/Part/App/PropertyTopoShapeList.cpp index 393d44d7d7..913f159b28 100644 --- a/src/Mod/Part/App/PropertyTopoShapeList.cpp +++ b/src/Mod/Part/App/PropertyTopoShapeList.cpp @@ -116,35 +116,37 @@ void PropertyTopoShapeList::afterRestore() PyObject *PropertyTopoShapeList::getPyObject() { - PyObject* list = PyList_New(getSize()); - for (int i = 0; i < getSize(); i++) - PyList_SetItem( list, i, _lValueList[i].getPyObject()); - return list; + Py::List list; + for (int i = 0; i < getSize(); i++) { + list.append(Py::asObject(_lValueList[i].getPyObject())); + } + return Py::new_reference_to(list); } void PropertyTopoShapeList::setPyObject(PyObject *value) { if (PySequence_Check(value)) { - Py_ssize_t nSize = PySequence_Size(value); + Py::Sequence sequence(value); + Py_ssize_t nSize = sequence.size(); std::vector values; values.resize(nSize); for (Py_ssize_t i=0; i < nSize; ++i) { - PyObject* item = PySequence_GetItem(value, i); - if (!PyObject_TypeCheck(item, &(TopoShapePy::Type))) { + Py::Object item = sequence.getItem(i); + if (!PyObject_TypeCheck(item.ptr(), &(TopoShapePy::Type))) { std::string error = std::string("types in list must be 'Shape', not "); - error += item->ob_type->tp_name; + error += item.ptr()->ob_type->tp_name; throw Base::TypeError(error); } - values[i] = *static_cast(item)->getTopoShapePtr(); + values[i] = *static_cast(item.ptr())->getTopoShapePtr(); } setValues(values); } else if (PyObject_TypeCheck(value, &(TopoShapePy::Type))) { TopoShapePy *pcObject = static_cast(value); setValue(*pcObject->getTopoShapePtr()); - } + } else { std::string error = std::string("type must be 'Shape' or list of 'Shape', not "); error += value->ob_type->tp_name; @@ -183,8 +185,7 @@ App::Property *PropertyTopoShapeList::Copy() const std::vector copiedShapes; for (auto& shape : _lValueList) { BRepBuilderAPI_Copy copy(shape.getShape()); - TopoDS_Shape* newShape = new TopoDS_Shape(copy.Shape()); - copiedShapes.emplace_back(*newShape); + copiedShapes.emplace_back(copy.Shape()); } p->setValues(copiedShapes); return p; diff --git a/src/Mod/Part/parttests/TopoShapeListTest.py b/src/Mod/Part/parttests/TopoShapeListTest.py index 22c93decb2..f594e86215 100644 --- a/src/Mod/Part/parttests/TopoShapeListTest.py +++ b/src/Mod/Part/parttests/TopoShapeListTest.py @@ -27,6 +27,7 @@ class TopoShapeListTest(unittest.TestCase): obj.Shapes = [box, box2, box3] doc.saveAs(self.fileName) App.closeDocument(doc.Name) + del obj, doc, box, box2, box3 print("TopoShapeListTest: setUp complete") def tearDown(self): diff --git a/src/Mod/Part/parttests/TopoShapeTest.py b/src/Mod/Part/parttests/TopoShapeTest.py index 8fc2150ea0..35e88a064c 100644 --- a/src/Mod/Part/parttests/TopoShapeTest.py +++ b/src/Mod/Part/parttests/TopoShapeTest.py @@ -367,6 +367,7 @@ class TopoShapeTest(unittest.TestCase, TopoShapeAssertions): path = Part.makeLine(App.Vector(), App.Vector(0, 0, 10)) Part.show(circle, "Circle") # Trigger the elementMapping Part.show(path, "Path") # Trigger the elementMapping + del circle # Act surface1 = Part.makeSweepSurface(self.doc.Path.Shape, self.doc.Circle.Shape, 0.001, 0) Part.show(surface1, "Sweep") @@ -387,6 +388,7 @@ class TopoShapeTest(unittest.TestCase, TopoShapeAssertions): # looks suspiciously like a substantial bug in OCCT. # Assert Shape self.assertBounds(surface1, App.BoundBox(-5, -2.72011, 0, 5, 5, 6.28319), precision=3) + del surface1 def testAppPartMakeLoft(self): # Act