Ensure ownership of path preprocessor is transferred

qgis · Jul 22, 2019 · 92707d8 · 92707d8
1 parent 5dc39fe
commit 92707d8
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 12 deletions.
diff --git a/python/core/auto_generated/qgspathresolver.sip.in b/python/core/auto_generated/qgspathresolver.sip.in
@@ -86,7 +86,7 @@ Example - replace stored database credentials with new ones:
 %End
 %MethodCode
     Py_BEGIN_ALLOW_THREADS
-
+    Py_XINCREF( a0 );
     QgsPathResolver::setPathPreprocessor( [a0]( const QString &arg )->QString
     {
       QString res;

diff --git a/src/core/qgspathresolver.h b/src/core/qgspathresolver.h
@@ -103,7 +103,7 @@ class CORE_EXPORT QgsPathResolver
     static void setPathPreprocessor( SIP_PYCALLABLE / AllowNone / );
     % MethodCode
     Py_BEGIN_ALLOW_THREADS
-
+    Py_XINCREF( a0 );
     QgsPathResolver::setPathPreprocessor( [a0]( const QString &arg )->QString
     {
       QString res;

diff --git a/tests/src/python/test_qgspathresolver.py b/tests/src/python/test_qgspathresolver.py
@@ -14,6 +14,7 @@
 
 import tempfile
 import os
+import gc
 from qgis.core import QgsPathResolver, QgsVectorLayer, QgsProject
 from qgis.testing import start_app, unittest
 from utilities import unitTestDataPath
@@ -22,20 +23,23 @@
 
 TEST_DATA_DIR = unitTestDataPath()
 
-PROCESSOR = None
-
 
 class TestQgsPathResolver(unittest.TestCase):
 
     def testCustomPreprocessor(self):
         self.assertEqual(QgsPathResolver().readPath('aaaaa'), 'aaaaa')
 
-        def my_processor(path):
-            return path.upper()
+        def run_test():
+            def my_processor(path):
+                return path.upper()
+
+            QgsPathResolver.setPathPreprocessor(my_processor)
+            self.assertEqual(QgsPathResolver().readPath('aaaaa'), 'AAAAA')
 
-        global PROCESSOR
-        PROCESSOR = my_processor
-        QgsPathResolver.setPathPreprocessor(PROCESSOR)
+        run_test()
+        gc.collect()
+        # my_processor should be out of scope and cleaned up, unless things are working
+        # correctly and ownership was transferred
         self.assertEqual(QgsPathResolver().readPath('aaaaa'), 'AAAAA')
 
     def testLoadLayerWithPreprocessor(self):
@@ -63,9 +67,7 @@ def testLoadLayerWithPreprocessor(self):
         def my_processor(path):
             return path.replace('moooooo', 'lines')
 
-        global PROCESSOR
-        PROCESSOR = my_processor
-        QgsPathResolver.setPathPreprocessor(PROCESSOR)
+        QgsPathResolver.setPathPreprocessor(my_processor)
         p3 = QgsProject()
         self.assertTrue(p3.read(temp_project_path))
         l = p3.mapLayersByName('Lines')[0]