From d11f7f55832358fb2d11f48d5bd119962c4b2ed1 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Fri, 21 Feb 2025 10:45:20 +1000 Subject: [PATCH] Fix leak when directly iterating feature in PyQGIS Fixes #32944 --- .../core/auto_generated/qgsfeature.sip.in | 3 ++ python/core/auto_generated/qgsfeature.sip.in | 3 ++ src/core/qgsfeature.h | 3 ++ tests/src/python/test_qgsvectorlayer.py | 33 +++++++++++++++++++ 4 files changed, 42 insertions(+) diff --git a/python/PyQt6/core/auto_generated/qgsfeature.sip.in b/python/PyQt6/core/auto_generated/qgsfeature.sip.in index 5cd75f42ce1..e624a019936 100644 --- a/python/PyQt6/core/auto_generated/qgsfeature.sip.in +++ b/python/PyQt6/core/auto_generated/qgsfeature.sip.in @@ -40,6 +40,9 @@ geometry and a list of field/values attributes. QgsAttributes attributes = sipCpp->attributes(); PyObject *attrs = sipConvertFromType( &attributes, sipType_QgsAttributes, Py_None ); sipRes = PyObject_GetIter( attrs ); + // PyObject_GetIter has added a ref to attrs - we need to decrement the ref from sipConvertFromType, + // so that the garbage collector will delete attrs when the iterator is deleted + Py_DECREF( attrs ); %End diff --git a/python/core/auto_generated/qgsfeature.sip.in b/python/core/auto_generated/qgsfeature.sip.in index 8b196f94a4c..e167301ff40 100644 --- a/python/core/auto_generated/qgsfeature.sip.in +++ b/python/core/auto_generated/qgsfeature.sip.in @@ -40,6 +40,9 @@ geometry and a list of field/values attributes. QgsAttributes attributes = sipCpp->attributes(); PyObject *attrs = sipConvertFromType( &attributes, sipType_QgsAttributes, Py_None ); sipRes = PyObject_GetIter( attrs ); + // PyObject_GetIter has added a ref to attrs - we need to decrement the ref from sipConvertFromType, + // so that the garbage collector will delete attrs when the iterator is deleted + Py_DECREF( attrs ); %End SIP_PYOBJECT __getitem__( int key ) /HoldGIL/; diff --git a/src/core/qgsfeature.h b/src/core/qgsfeature.h index 0056d0489a0..bba466c1b09 100644 --- a/src/core/qgsfeature.h +++ b/src/core/qgsfeature.h @@ -76,6 +76,9 @@ class CORE_EXPORT QgsFeature QgsAttributes attributes = sipCpp->attributes(); PyObject *attrs = sipConvertFromType( &attributes, sipType_QgsAttributes, Py_None ); sipRes = PyObject_GetIter( attrs ); + // PyObject_GetIter has added a ref to attrs - we need to decrement the ref from sipConvertFromType, + // so that the garbage collector will delete attrs when the iterator is deleted + Py_DECREF( attrs ); % End #endif diff --git a/tests/src/python/test_qgsvectorlayer.py b/tests/src/python/test_qgsvectorlayer.py index 1ec7dbff81a..d40e86353fd 100644 --- a/tests/src/python/test_qgsvectorlayer.py +++ b/tests/src/python/test_qgsvectorlayer.py @@ -320,6 +320,39 @@ class TestQgsVectorLayer(QgisTestCase, FeatureSourceTestCase): myCount = myLayer.featureCount() self.assertEqual(myCount, 6) + def test_attribute_iteration(self): + layer = QgsVectorLayer( + self.get_test_data_path("lines.shp").as_posix(), "Lines", "ogr" + ) + self.assertTrue(layer.isValid()) + all_attrs = [ + [attr for attr in feat.attributes()] for feat in layer.getFeatures() + ] + self.assertCountEqual( + all_attrs, + [ + ["Highway", 1.0], + ["Highway", 1.0], + ["Arterial", 2.0], + ["Arterial", 2.0], + ["Arterial", 2.0], + ["Arterial", 2.0], + ], + ) + + all_attrs = [[attr for attr in feat] for feat in layer.getFeatures()] + self.assertCountEqual( + all_attrs, + [ + ["Highway", 1.0], + ["Highway", 1.0], + ["Arterial", 2.0], + ["Arterial", 2.0], + ["Arterial", 2.0], + ["Arterial", 2.0], + ], + ) + # undo stack def testUndoStack(self): layer = createLayerWithOnePoint()