Fix crashes during and after PythonQt cleanup

.github/workflows/build.yml

@@ -82,6 +82,22 @@ jobs:
         UBSAN_OPTIONS="halt_on_error=1" ASAN_OPTIONS="detect_leaks=1:detect_stack_use_after_return=1:fast_unwind_on_malloc=0" \
         PYTHONQT_RUN_ONLY_MEMORY_TESTS=1 \
           make check TESTARGS="-platform minimal"
+
+    - name: Run cleanup tests with sanitizers
+      run: |
+        # CPython 3.12 only: suppress its known interned-unicode leak that
+        # shows up as allocations via PyUnicode_New (python/cpython#113190).
+        # Fixed in 3.13 (python/cpython#113601), so we scope this to 3.12.
+        PYVER=$(python3 -c 'import sys; print(".".join(map(str, sys.version_info[:2])))')
+        if [[ "$PYVER" == "3.12" ]]; then
+          echo "leak:PyUnicode_New" >> $PWD/lsan.supp
+          export LSAN_OPTIONS="suppressions=$PWD/lsan.supp"
+        fi
+        PYTHONDEVMODE=1 PYTHONASYNCIODEBUG=1 PYTHONWARNINGS=error PYTHONMALLOC=malloc_debug \
+        UBSAN_OPTIONS="halt_on_error=1" ASAN_OPTIONS="detect_leaks=1:detect_stack_use_after_return=1:fast_unwind_on_malloc=0" \
+        PYTHONQT_RUN_ONLY_CLEANUP_TESTS=1 \
+        PYTHONQT_DISABLE_ASYNCIO=1 \
+          make check TESTARGS="-platform minimal"
 
     - name: Generate Wrappers
       run: |

src/PythonQt.cpp

@@ -102,12 +102,17 @@ void PythonQt::init(int flags, const QByteArray& pythonQtModuleName)
     }
 
 #ifdef PY3K
-    PythonQtObjectPtr asyncio;
-    asyncio.setNewRef(PyImport_ImportModule("asyncio"));
-    if (asyncio)
-    {
-      _self->_p->_pyEnsureFuture = asyncio.getVariable("ensure_future");
-      _self->_p->_pyFutureClass = asyncio.getVariable("Future");
+    // Import asyncio only when not explicitly disabled.
+    // Importing asyncio on Py3.12+ pulls in ssl/_ssl; some environments/tests
+    // want to avoid that during early embedded init.
+    if (!qEnvironmentVariableIsSet("PYTHONQT_DISABLE_ASYNCIO")) {
+        PythonQtObjectPtr asyncio;
+        asyncio.setNewRef(PyImport_ImportModule("asyncio"));
+        if (asyncio)
+        {
+          _self->_p->_pyEnsureFuture = asyncio.getVariable("ensure_future");
+          _self->_p->_pyFutureClass = asyncio.getVariable("Future");
+        }
     }
 #endif
 
@@ -326,6 +331,10 @@ void PythonQt::init(int flags, const QByteArray& pythonQtModuleName)
 void PythonQt::cleanup()
 {
   if (_self) {
+    // Remove signal handlers in advance, since destroying them calls back into
+    // PythonQt::priv()->removeSignalEmitter()
+    _self->removeSignalHandlers();
+
     delete _self;
     _self = nullptr;
   }
@@ -417,9 +426,10 @@ PythonQtPrivate::~PythonQtPrivate() {
   delete _defaultImporter;
   _defaultImporter = nullptr;
 
-  {
-    qDeleteAll(_knownClassInfos);
-  }
+  PythonQtClassInfo::clearGlobalNamespaceWrappers();
+
+  qDeleteAll(_knownClassInfos);
+  _knownClassInfos.clear();
 
   PythonQtMethodInfo::cleanupCachedMethodInfos();
   PythonQtArgumentFrame::cleanupFreeList();

src/PythonQt.h

@@ -566,7 +566,7 @@ class PYTHONQT_EXPORT PythonQt : public QObject {
   //@{
 
   //! get access to internal data (should not be used on the public API, but is used by some C functions)
-  static PythonQtPrivate* priv() { return _self->_p; }
+  static PythonQtPrivate* priv() { return _self ? _self->_p : nullptr; }
 
   //! clear all NotFound entries on all class infos, to ensure that
   //! newly loaded wrappers can add methods even when the object was wrapped by PythonQt before the wrapper was loaded

src/PythonQtClassInfo.cpp

@@ -1043,6 +1043,11 @@ void PythonQtClassInfo::addGlobalNamespaceWrapper(PythonQtClassInfo* namespaceWr
   _globalNamespaceWrappers.insert(0, namespaceWrapper);
 }
 
+void PythonQtClassInfo::clearGlobalNamespaceWrappers()
+{
+  _globalNamespaceWrappers.clear();
+}
+
 void PythonQtClassInfo::updateRefCountingCBs()
 {
   if (!_refCallback) {

src/PythonQtClassInfo.h

@@ -244,6 +244,10 @@ class PYTHONQT_EXPORT PythonQtClassInfo {
   //! Add a wrapper that contains global enums
   static void addGlobalNamespaceWrapper(PythonQtClassInfo* namespaceWrapper);
 
+  //! Clear the registry of global-namespace wrappers (used for top-level enums).
+  //! Must be called before destroying PythonQtClassInfo instances and before a fresh init.
+  static void clearGlobalNamespaceWrappers();
+
 private:
   void updateRefCountingCBs();
 

src/PythonQtInstanceWrapper.cpp

@@ -127,7 +127,9 @@ static void PythonQtInstanceWrapper_deleteObject(PythonQtInstanceWrapper* self,
 
 static void PythonQtInstanceWrapper_dealloc(PythonQtInstanceWrapper* self)
 {
-  PythonQtInstanceWrapper_deleteObject(self);
+  if (PythonQt::self()) {
+    PythonQtInstanceWrapper_deleteObject(self);
+  }
   self->_obj.~QPointer<QObject>();
   Py_TYPE(self)->tp_free((PyObject*)self);
 }
@@ -221,6 +223,9 @@ int PythonQtInstanceWrapper_init(PythonQtInstanceWrapper * self, PyObject * args
 
 static PyObject *PythonQtInstanceWrapper_richcompare(PythonQtInstanceWrapper* wrapper, PyObject* other, int code)
 {
+  if (PythonQt::self() == nullptr || PythonQt::priv() == nullptr) {
+    Py_RETURN_NOTIMPLEMENTED;
+  }
   bool validPtrs = false;
   bool areSamePtrs = false;
   if (PyObject_TypeCheck((PyObject*)wrapper, &PythonQtInstanceWrapper_Type)) {
@@ -333,6 +338,10 @@ static PyObject *PythonQtInstanceWrapper_classname(PythonQtInstanceWrapper* obj)
 
 PyObject *PythonQtInstanceWrapper_inherits(PythonQtInstanceWrapper* obj, PyObject *args)
 {
+  if (PythonQt::self() == nullptr || PythonQt::priv() == nullptr) {
+    PyErr_SetString(PyExc_RuntimeError, "PythonQt is not initialized (or has been finalized)");
+    return nullptr;
+  }
   char *name = nullptr;
   if (!PyArg_ParseTuple(args, "s:PythonQtInstanceWrapper.inherits",&name)) {
     return nullptr;
@@ -342,11 +351,17 @@ PyObject *PythonQtInstanceWrapper_inherits(PythonQtInstanceWrapper* obj, PyObjec
 
 static PyObject *PythonQtInstanceWrapper_help(PythonQtInstanceWrapper* obj)
 {
+  if (PythonQt::self() == nullptr || PythonQt::priv() == nullptr) {
+    Py_RETURN_NONE;
+  }
   return PythonQt::self()->helpCalled(obj->classInfo());
 }
 
 PyObject *PythonQtInstanceWrapper_delete(PythonQtInstanceWrapper * self)
 {
+  if (PythonQt::self() == nullptr || PythonQt::priv() == nullptr) {
+    Py_RETURN_NONE;
+  }
   PythonQtMemberInfo deleteSlot = self->classInfo()->member("py_delete");
   if (deleteSlot._type == PythonQtMemberInfo::Slot) {
     // call the py_delete slot instead of internal C++ destructor...
@@ -378,6 +393,9 @@ static PyMethodDef PythonQtInstanceWrapper_methods[] = {
 
 static PyObject *PythonQtInstanceWrapper_getattro(PyObject *obj,PyObject *name)
 {
+  if (PythonQt::self() == nullptr || PythonQt::priv() == nullptr) {
+    return PyObject_GenericGetAttr(obj, name);
+  }
   const char *attributeName;
   PythonQtInstanceWrapper *wrapper = (PythonQtInstanceWrapper *)obj;
 
@@ -609,6 +627,10 @@ static PyObject *PythonQtInstanceWrapper_getattro(PyObject *obj,PyObject *name)
 
 static int PythonQtInstanceWrapper_setattro(PyObject *obj,PyObject *name,PyObject *value)
 {
+  if (PythonQt::self() == nullptr || PythonQt::priv() == nullptr) {
+    PyErr_SetString(PyExc_AttributeError, "PythonQt is not initialized (or has been finalized); cannot set attributes on this wrapper");
+    return -1;
+  }
   QString error;
   const char *attributeName;
   PythonQtInstanceWrapper *wrapper = (PythonQtInstanceWrapper *)obj;
@@ -761,6 +783,9 @@ static QString getStringFromObject(PythonQtInstanceWrapper* wrapper) {
 
 static PyObject * PythonQtInstanceWrapper_str(PyObject * obj)
 {
+  if (PythonQt::self() == nullptr || PythonQt::priv() == nullptr) {
+    return PyUnicode_New(0, 0);
+  }
   PythonQtInstanceWrapper* wrapper = (PythonQtInstanceWrapper*)obj;
 
   // QByteArray should be directly returned as a str
@@ -806,6 +831,9 @@ static PyObject * PythonQtInstanceWrapper_str(PyObject * obj)
 
 static PyObject * PythonQtInstanceWrapper_repr(PyObject * obj)
 {
+  if (PythonQt::self() == nullptr || PythonQt::priv() == nullptr) {
+    return PyUnicode_New(0, 0);
+  }
   PythonQtInstanceWrapper* wrapper = (PythonQtInstanceWrapper*)obj;
   const char* typeName = obj->ob_type->tp_name;
 

src/PythonQtObjectPtr.cpp

@@ -109,7 +109,7 @@ PythonQtObjectPtr::PythonQtObjectPtr(PythonQtSafeObjectPtr &&p) :_object(p.takeO
 
 PythonQtObjectPtr::~PythonQtObjectPtr()
 {
-  Py_XDECREF(_object);
+  if (Py_IsInitialized()) Py_XDECREF(_object);
 }
 
 void PythonQtObjectPtr::setNewRef(PyObject* o)
@@ -172,7 +172,7 @@ PythonQtSafeObjectPtr::~PythonQtSafeObjectPtr()
 {
   if (_object) {
     PYTHONQT_GIL_SCOPE
-    Py_DECREF(_object);
+    if (Py_IsInitialized()) Py_DECREF(_object);
   }
 }
 

src/PythonQtSignalReceiver.cpp

@@ -178,10 +178,12 @@ PythonQtSignalReceiver::PythonQtSignalReceiver(QObject* obj):PythonQtSignalRecei
 
 PythonQtSignalReceiver::~PythonQtSignalReceiver()
 {
-  // we need the GIL scope here, because the targets keep references to Python objects
-  PYTHONQT_GIL_SCOPE;
-  PythonQt::priv()->removeSignalEmitter(_obj);
-  _targets.clear();
+  if (PythonQt::priv()) {
+    // we need the GIL scope here, because the targets keep references to Python objects
+    PYTHONQT_GIL_SCOPE;
+    PythonQt::priv()->removeSignalEmitter(_obj);
+    _targets.clear();
+  }
 }
 
 

tests/PythonQtTestCleanup.cpp

@@ -0,0 +1,96 @@
+#include "PythonQtTestCleanup.h"
+#include "PythonQt.h"
+#include "PythonQt_QtAll.h"
+
+void PythonQtTestCleanup::initTestCase()
+{
+}
+
+void PythonQtTestCleanup::cleanupTestCase()
+{
+}
+
+void PythonQtTestCleanup::init()
+{
+  // Initialize before each test
+
+  PythonQt::init(PythonQt::IgnoreSiteModule);
+  PythonQt_QtAll::init();
+
+  _helper = new PythonQtTestCleanupHelper();
+  PythonQtObjectPtr main = PythonQt::self()->getMainModule();
+  PythonQt::self()->addObject(main, "obj", _helper);
+}
+
+void PythonQtTestCleanup::cleanup()
+{
+  // Cleanup PythonQt resources before finalizing Python
+  PythonQt::cleanup();
+
+  if (Py_IsInitialized()) {
+    Py_Finalize();
+  }
+
+  delete _helper;
+  _helper = nullptr;
+}
+
+void PythonQtTestCleanup::testQtEnum()
+{
+  QVERIFY(_helper->runScript(
+    "import PythonQt.QtCore\n" \
+    "x = PythonQt.QtCore.QFile.ReadOnly\n" \
+    "obj.setPassed()"
+    ));
+}
+
+void PythonQtTestCleanup::testCallQtMethodInDestructorOwnedQTimer()
+{
+  QVERIFY(_helper->runScript(
+    "import PythonQt.QtCore\n" \
+    "class TimerWrapper(object):\n" \
+    "  def __init__(self):\n" \
+    "    self.timer = PythonQt.QtCore.QTimer()\n" \
+    "  def __del__(self):\n" \
+    "    self.timer.setSingleShot(True)\n" \
+    "x = TimerWrapper()\n" \
+    "del x\n" \
+    "obj.setPassed()\n"
+    ));
+}
+
+void PythonQtTestCleanup::testCallQtMethodInDestructorWeakRefGuarded()
+{
+    QVERIFY(_helper->runScript(
+        "import weakref\n" \
+        "import PythonQt.QtCore\n" \
+        "class TimerWrapper(object):\n" \
+        "  def __init__(self):\n" \
+        "    self.timerWeakRef = weakref.ref(PythonQt.QtCore.QTimer())\n" \
+        "  def __del__(self):\n" \
+        "    if self.timerWeakRef():\n" \
+        "      self.timerWeakRef().setSingleShot(True)\n" \
+        "x = TimerWrapper()\n" \
+        "obj.setPassed()\n"
+        ));
+}
+
+void PythonQtTestCleanup::testSignalReceiverCleanup()
+{
+  PythonQtObjectPtr main = PythonQt::self()->getMainModule();
+
+  // Test that PythonQtSignalReceiver is cleaned up properly,
+  // i.e. PythonQt::cleanup() doesn't segfault
+  main.evalScript(
+    "import PythonQt.QtCore\n" \
+    "timer = PythonQt.QtCore.QTimer(obj)\n" \
+    "timer.connect('destroyed()', obj.onDestroyed)\n"
+    );
+}
+
+bool PythonQtTestCleanupHelper::runScript(const char* script)
+{
+  _passed = false;
+  PyRun_SimpleString(script);
+  return _passed;
+}

tests/PythonQtTestCleanup.h

@@ -0,0 +1,47 @@
+#ifndef _PYTHONQTTESTCLEANUP_H
+#define _PYTHONQTTESTCLEANUP_H
+
+#include <QtTest/QtTest>
+
+class PythonQtTestCleanupHelper;
+
+//! Test PythonQt cleanup and Python interpreter finalization
+class PythonQtTestCleanup : public QObject
+{
+  Q_OBJECT
+
+private Q_SLOTS:
+  void initTestCase();
+  void cleanupTestCase();
+  void init();
+  void cleanup();
+
+  void testQtEnum();
+  void testCallQtMethodInDestructorOwnedQTimer();
+  void testCallQtMethodInDestructorWeakRefGuarded();
+  void testSignalReceiverCleanup();
+
+private:
+  PythonQtTestCleanupHelper* _helper;
+};
+
+//! Test helper class
+class PythonQtTestCleanupHelper : public QObject
+{
+  Q_OBJECT
+public:
+  PythonQtTestCleanupHelper() :
+    _passed(false) {
+  };
+
+  bool runScript(const char* script);
+
+public Q_SLOTS:
+  void setPassed() { _passed = true; }
+  void onDestroyed(QObject *) { }
+
+private:
+  bool _passed;
+};
+
+#endif