Fix ownership logic for PythonHandle objects

Cantera · Jun 11, 2023 · 13cb334 · 13cb334
1 parent 3f24588
commit 13cb334
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 5 deletions.
diff --git a/include/cantera/extensions/PythonHandle.h b/include/cantera/extensions/PythonHandle.h
@@ -20,8 +20,14 @@ class PythonHandle : public ExternalHandle
     //! @param obj  The Python object to be held
     //! @param weak  `true` if this is a weak reference to the Python object and this
     //!    handle is not responsible for deleting the Python object, or `false` if this
-    //!    handle "owns" the Python object
-    PythonHandle(PyObject* obj, bool weak) : m_obj(obj), m_weak(weak) {}
+    //!    handle should own a reference to the Python object
+    PythonHandle(PyObject* obj, bool weak) : m_obj(obj), m_weak(weak) {
+        if (!weak) {
+            Py_XINCREF(obj);
+        }
+    }
+    PythonHandle(const PythonHandle&) = delete;
+    PythonHandle& operator=(const PythonHandle&) = delete;
 
     ~PythonHandle() {
         if (!m_weak) {

diff --git a/interfaces/cython/cantera/solutionbase.pyx b/interfaces/cython/cantera/solutionbase.pyx
@@ -463,7 +463,7 @@ cdef _assign_Solution(_SolutionBase soln, shared_ptr[CxxSolution] cxx_soln,
             soln._adjacent[name] = _wrap_Solution(adj_soln)
 
     cdef shared_ptr[CxxExternalHandle] handle
-    handle.reset(new CxxPythonHandle(<PyObject*>soln, True))
+    handle.reset(new CxxPythonHandle(<PyObject*>soln, not weak))
     soln.base.holdExternalHandle(stringify("python"), handle)
 
 

diff --git a/src/extensions/PythonExtensionManager.cpp b/src/extensions/PythonExtensionManager.cpp
@@ -103,6 +103,7 @@ void PythonExtensionManager::registerRateBuilder(
         // reference cycle is handled on the Python side.
         delegator->holdExternalHandle("python",
                                       make_shared<PythonHandle>(extRate, false));
+        Py_XDECREF(extRate);
         return delegator.release();
     };
     ReactionRateFactory::factory()->reg(rateName, builder);
@@ -124,7 +125,9 @@ void PythonExtensionManager::registerRateDataBuilder(
                                "Problem in ct_newPythonExtensibleRateData:\n{}",
                                getPythonExceptionInfo());
         }
-        delegator.setWrapper(make_shared<PythonHandle>(extData, false));
+        auto handle = make_shared<PythonHandle>(extData, false);
+        Py_XDECREF(extData);
+        delegator.setWrapper(handle);
     };
     mgr.registerReactionDataLinker(rateName, "python", builder);
 
@@ -137,7 +140,9 @@ void PythonExtensionManager::registerRateDataBuilder(
                                "Problem in ct_wrapSolution:\n{}",
                                getPythonExceptionInfo());
         }
-        return make_shared<PythonHandle>(pySoln, false);
+        auto handle = make_shared<PythonHandle>(pySoln, false);
+        Py_XDECREF(pySoln);
+        return handle;
     };
     mgr.registerSolutionLinker("python", solnLinker);
 }