From 10bbc3fa244912dc4b1e5d1f21fe10e51dc66268 Mon Sep 17 00:00:00 2001 From: Max Smolens Date: Fri, 11 Sep 2015 09:17:59 -0400 Subject: [PATCH 01/14] Prevent crashes during and after cleanup This commit prevents crashes by handling scenarios such as: (a) object destruction after the Python interpreter has been finalized (b) object destruction after cleanup, i.e. the singleton no longer exists Any usage of a Qt enum demonstrates (a). One example that demonstrates (b) is a QTimer object which is created with a QApplication parent. PythonQt::cleanup() is called before the QApplication is completely destroyed, so the code that handles wrapping the QTimer object must handle the case when the PythonQt singleton no longer exists. (cherry picked from commit commontk/PythonQt@bf07cc8a5e5fafff677b958e01ad5f553459084b) Co-authored-by: Jean-Christophe Fillion-Robin --- src/PythonQt.h | 2 +- src/PythonQtObjectPtr.cpp | 2 +- src/PythonQtSignalReceiver.cpp | 10 ++++++---- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/PythonQt.h b/src/PythonQt.h index 59b17557a..155299e3d 100644 --- a/src/PythonQt.h +++ b/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 diff --git a/src/PythonQtObjectPtr.cpp b/src/PythonQtObjectPtr.cpp index 21cd8c1a0..c0629483b 100644 --- a/src/PythonQtObjectPtr.cpp +++ b/src/PythonQtObjectPtr.cpp @@ -109,7 +109,7 @@ PythonQtObjectPtr::PythonQtObjectPtr(PythonQtSafeObjectPtr &&p) :_object(p.takeO PythonQtObjectPtr::~PythonQtObjectPtr() { - Py_XDECREF(_object); + if (_object && Py_IsInitialized()) Py_XDECREF(_object); } void PythonQtObjectPtr::setNewRef(PyObject* o) diff --git a/src/PythonQtSignalReceiver.cpp b/src/PythonQtSignalReceiver.cpp index d2a04efea..2fe6e4b00 100644 --- a/src/PythonQtSignalReceiver.cpp +++ b/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(); + } } From c00824fa147667b4bc6544233fffa2d60a5fa6b8 Mon Sep 17 00:00:00 2001 From: Max Smolens Date: Fri, 11 Sep 2015 16:36:51 -0400 Subject: [PATCH 02/14] Add cleanup/finalization tests Introduce tests to verify proper cleanup and finalization across different scenarios. Backport notes: - Adapted to include `PythonQt_QtAll.h` and call `PythonQt_QtAll::init()` instead of `PythonQt_QtBindings.h` and `PythonQt_init_QtBindings()`, which are specific to the commontk fork. - Replaced NULL with nullptr and remove unused `PythonQt.h` include from `PythonQtTestCleanup.h`. (cherry picked from commit commontk/PythonQt@4ad7e74bf9c74a6659dacb61d0296a6c8e77fd7d) Co-authored-by: Jean-Christophe Fillion-Robin Co-authored-by: Pat Marion --- tests/PythonQtTestCleanup.cpp | 69 +++++++++++++++++++++++++++++++++++ tests/PythonQtTestCleanup.h | 44 ++++++++++++++++++++++ tests/PythonQtTestMain.cpp | 8 ++++ tests/tests.pro | 3 ++ 4 files changed, 124 insertions(+) create mode 100644 tests/PythonQtTestCleanup.cpp create mode 100644 tests/PythonQtTestCleanup.h diff --git a/tests/PythonQtTestCleanup.cpp b/tests/PythonQtTestCleanup.cpp new file mode 100644 index 000000000..91bbac2b1 --- /dev/null +++ b/tests/PythonQtTestCleanup.cpp @@ -0,0 +1,69 @@ +#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() +{ + // Finalize and cleanup after each test + + PythonQtObjectPtr main = PythonQt::self()->getMainModule(); + PythonQt::self()->removeVariable(main, "obj"); + delete _helper; + _helper = nullptr; + + if (Py_IsInitialized()) { + Py_Finalize(); + } + + PythonQt::cleanup(); +} + +void PythonQtTestCleanup::testQtEnum() +{ + QVERIFY(_helper->runScript( + "import PythonQt.QtCore\n" \ + "x = PythonQt.QtCore.QFile.ReadOnly\n" \ + "obj.setPassed()" + )); +} + +void PythonQtTestCleanup::testCallQtMethodInDel() +{ + 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" \ + "obj.setPassed()\n" + )); +} + +bool PythonQtTestCleanupHelper::runScript(const char* script) +{ + _passed = false; + PyRun_SimpleString(script); + return _passed; +} diff --git a/tests/PythonQtTestCleanup.h b/tests/PythonQtTestCleanup.h new file mode 100644 index 000000000..c7b1b28aa --- /dev/null +++ b/tests/PythonQtTestCleanup.h @@ -0,0 +1,44 @@ +#ifndef _PYTHONQTTESTCLEANUP_H +#define _PYTHONQTTESTCLEANUP_H + +#include + +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 testCallQtMethodInDel(); + +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; } + +private: + bool _passed; +}; + +#endif diff --git a/tests/PythonQtTestMain.cpp b/tests/PythonQtTestMain.cpp index 051301465..b3e4f3559 100644 --- a/tests/PythonQtTestMain.cpp +++ b/tests/PythonQtTestMain.cpp @@ -41,6 +41,7 @@ #include "PythonQt.h" #include "PythonQtTests.h" +#include "PythonQtTestCleanup.h" #include @@ -65,6 +66,13 @@ int main( int argc, char **argv ) PythonQt::cleanup(); + if (Py_IsInitialized()) { + Py_Finalize(); + } + + PythonQtTestCleanup cleanup; + failCount += QTest::qExec(&cleanup, argc, argv); + if (failCount) { std::cerr << "Tests failed: " << failCount << std::endl; } else { diff --git a/tests/tests.pro b/tests/tests.pro index fae2c7bd1..90a72ce07 100644 --- a/tests/tests.pro +++ b/tests/tests.pro @@ -23,10 +23,13 @@ QT += widgets include ( ../build/common.prf ) include ( ../build/PythonQt.prf ) +include ( ../build/PythonQt_QtAll.prf ) HEADERS += \ + PythonQtTestCleanup.h \ PythonQtTests.h SOURCES += \ + PythonQtTestCleanup.cpp \ PythonQtTestMain.cpp \ PythonQtTests.cpp From 11590ea449c368413c6e5f19d51836311b91cab4 Mon Sep 17 00:00:00 2001 From: Max Smolens Date: Tue, 15 Sep 2015 15:45:30 -0400 Subject: [PATCH 03/14] Fix PythonQtSignalReceiver crash during cleanup This commit fixes a crash during PythonQt::cleanup(). While destroying the PythonQtPrivate instance, any remaining PythonQtSignalReceivers that are kept alive only by their parent object will be destroyed. The crash occurs when PythonQtSignalReceiver's destructor calls back into PythonQtPrivate::removeSignalEmitter() when the PythonQtPrivate instance is mostly destroyed. Includes test case that crashes without the fix. (cherry picked from commit commontk/PythonQt@f0ea5bf787821a5d6d2793de56e406db8b32d242) --- src/PythonQt.cpp | 4 ++++ tests/PythonQtTestCleanup.cpp | 21 ++++++++++++++++----- tests/PythonQtTestCleanup.h | 2 ++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/PythonQt.cpp b/src/PythonQt.cpp index f6c0e05c4..87b128b66 100644 --- a/src/PythonQt.cpp +++ b/src/PythonQt.cpp @@ -326,6 +326,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; } diff --git a/tests/PythonQtTestCleanup.cpp b/tests/PythonQtTestCleanup.cpp index 91bbac2b1..a88b66c3b 100644 --- a/tests/PythonQtTestCleanup.cpp +++ b/tests/PythonQtTestCleanup.cpp @@ -26,16 +26,14 @@ void PythonQtTestCleanup::cleanup() { // Finalize and cleanup after each test - PythonQtObjectPtr main = PythonQt::self()->getMainModule(); - PythonQt::self()->removeVariable(main, "obj"); - delete _helper; - _helper = nullptr; - if (Py_IsInitialized()) { Py_Finalize(); } PythonQt::cleanup(); + + delete _helper; + _helper = nullptr; } void PythonQtTestCleanup::testQtEnum() @@ -61,6 +59,19 @@ void PythonQtTestCleanup::testCallQtMethodInDel() )); } +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; diff --git a/tests/PythonQtTestCleanup.h b/tests/PythonQtTestCleanup.h index c7b1b28aa..a7c73b665 100644 --- a/tests/PythonQtTestCleanup.h +++ b/tests/PythonQtTestCleanup.h @@ -18,6 +18,7 @@ private Q_SLOTS: void testQtEnum(); void testCallQtMethodInDel(); + void testSignalReceiverCleanup(); private: PythonQtTestCleanupHelper* _helper; @@ -36,6 +37,7 @@ class PythonQtTestCleanupHelper : public QObject public Q_SLOTS: void setPassed() { _passed = true; } + void onDestroyed(QObject *) { } private: bool _passed; From 6407969d11062d85950db2c54b78e4d9ed10f189 Mon Sep 17 00:00:00 2001 From: James Butler Date: Thu, 14 Dec 2023 13:13:07 -0500 Subject: [PATCH 04/14] Prevent crashes during and after cleanup (cont'd) This is a follow-up to HEAD~3 ("Prevent crashes during and after cleanup") (cherry picked from commit commontk/PythonQt@8aa183b243b5085620ab859990362c510e0a482c) --- src/PythonQtObjectPtr.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PythonQtObjectPtr.cpp b/src/PythonQtObjectPtr.cpp index c0629483b..98d877a4d 100644 --- a/src/PythonQtObjectPtr.cpp +++ b/src/PythonQtObjectPtr.cpp @@ -172,7 +172,7 @@ PythonQtSafeObjectPtr::~PythonQtSafeObjectPtr() { if (_object) { PYTHONQT_GIL_SCOPE - Py_DECREF(_object); + if (_object && Py_IsInitialized()) Py_DECREF(_object); } } From a8572b3fc6d71a1019cc61f1901642eb8cc24b6b Mon Sep 17 00:00:00 2001 From: Max Smolens Date: Fri, 28 Aug 2015 14:14:58 -0400 Subject: [PATCH 05/14] Prevent crash when an object is destroyed after calling PythonQt::cleanup() In certain situations the dealloc callback `PythonQtInstanceWrapper_dealloc` is called after `PythonQt::cleanup()` has been run. This can happen when Python destroys objects during `Py_Finalize()`. This commit adds a check that PythonQt is still initialized in the dealloc callback. (cherry picked from commit commontk/PythonQt@e4bd663b47ea3257001f357cfc7e3531843aaa0e) --- src/PythonQtInstanceWrapper.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/PythonQtInstanceWrapper.cpp b/src/PythonQtInstanceWrapper.cpp index d45b627f5..19e765e40 100644 --- a/src/PythonQtInstanceWrapper.cpp +++ b/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(); Py_TYPE(self)->tp_free((PyObject*)self); } From 9bc4221b88b58eed8a7b1718069f491a0b17051c Mon Sep 17 00:00:00 2001 From: Jean-Christophe Fillion-Robin Date: Mon, 1 Sep 2025 17:56:01 -0400 Subject: [PATCH 06/14] chore(PythonQtObjectPtr): Remove redundant nullptr checks Suggested-by: Uwe Siems --- src/PythonQtObjectPtr.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/PythonQtObjectPtr.cpp b/src/PythonQtObjectPtr.cpp index 98d877a4d..8a214ec81 100644 --- a/src/PythonQtObjectPtr.cpp +++ b/src/PythonQtObjectPtr.cpp @@ -109,7 +109,7 @@ PythonQtObjectPtr::PythonQtObjectPtr(PythonQtSafeObjectPtr &&p) :_object(p.takeO PythonQtObjectPtr::~PythonQtObjectPtr() { - if (_object && Py_IsInitialized()) 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 - if (_object && Py_IsInitialized()) Py_DECREF(_object); + if (Py_IsInitialized()) Py_DECREF(_object); } } From e778d6b31fb94d4e3da75355512ff95925e5ce38 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Fillion-Robin Date: Tue, 2 Sep 2025 00:18:04 -0400 Subject: [PATCH 07/14] fix(cleanup): clear global enum-wrapper registry before tearing down class infos ASan reported a heap-use-after-free during teardown: the global registry of top-level enum wrappers held raw pointers to `PythonQtClassInfo` that were deleted in `~PythonQtPrivate()`. Subsequent lookups (e.g., triggered by late attribute access) read freed memory. - Add `PythonQtClassInfo::clearGlobalNamespaceWrappers()` - Call it in `~PythonQtPrivate()` before `qDeleteAll(_knownClassInfos)` - Explicitly clear `_knownClassInfos` after deletion to avoid stale entries --- src/PythonQt.cpp | 7 ++++--- src/PythonQtClassInfo.cpp | 5 +++++ src/PythonQtClassInfo.h | 4 ++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/PythonQt.cpp b/src/PythonQt.cpp index 87b128b66..387f7d508 100644 --- a/src/PythonQt.cpp +++ b/src/PythonQt.cpp @@ -421,9 +421,10 @@ PythonQtPrivate::~PythonQtPrivate() { delete _defaultImporter; _defaultImporter = nullptr; - { - qDeleteAll(_knownClassInfos); - } + PythonQtClassInfo::clearGlobalNamespaceWrappers(); + + qDeleteAll(_knownClassInfos); + _knownClassInfos.clear(); PythonQtMethodInfo::cleanupCachedMethodInfos(); PythonQtArgumentFrame::cleanupFreeList(); diff --git a/src/PythonQtClassInfo.cpp b/src/PythonQtClassInfo.cpp index b730e982d..af5e5c0c7 100644 --- a/src/PythonQtClassInfo.cpp +++ b/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) { diff --git a/src/PythonQtClassInfo.h b/src/PythonQtClassInfo.h index fa439f5df..04f1d46b4 100644 --- a/src/PythonQtClassInfo.h +++ b/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(); From caa112992913cb80267d871b06001bee550e8302 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Fillion-Robin Date: Tue, 2 Sep 2025 00:19:17 -0400 Subject: [PATCH 08/14] fix: guard InstanceWrapper during cleanup and Py_Finalize During interpreter shutdown, Python objects may still invoke wrapper methods after `PythonQt::cleanup()` has nullified internals, leading to "heap-use-after-free". This commit adds guards and fallbacks. --- src/PythonQtInstanceWrapper.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/PythonQtInstanceWrapper.cpp b/src/PythonQtInstanceWrapper.cpp index 19e765e40..41a3e5560 100644 --- a/src/PythonQtInstanceWrapper.cpp +++ b/src/PythonQtInstanceWrapper.cpp @@ -223,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)) { @@ -335,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; @@ -344,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... @@ -380,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; @@ -611,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; @@ -763,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 @@ -808,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; From a0221d6714332b701e51c3a3842160ca0ce5768e Mon Sep 17 00:00:00 2001 From: Jean-Christophe Fillion-Robin Date: Tue, 2 Sep 2025 01:02:43 -0400 Subject: [PATCH 09/14] tests: isolate cleanup tests --- .github/workflows/build.yml | 7 +++++++ tests/PythonQtTestMain.cpp | 9 ++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 97e2dc080..5ced57f91 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -82,6 +82,13 @@ 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: | + 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 \ + make check TESTARGS="-platform minimal" - name: Generate Wrappers run: | diff --git a/tests/PythonQtTestMain.cpp b/tests/PythonQtTestMain.cpp index b3e4f3559..f8e82c833 100644 --- a/tests/PythonQtTestMain.cpp +++ b/tests/PythonQtTestMain.cpp @@ -55,6 +55,12 @@ int main( int argc, char **argv ) return 0; } + if (QProcessEnvironment::systemEnvironment().contains("PYTHONQT_RUN_ONLY_CLEANUP_TESTS")) { + PythonQtTestCleanup cleanup; + QTest::qExec(&cleanup, argc, argv); + return 0; + } + PythonQt::init(PythonQt::IgnoreSiteModule | PythonQt::RedirectStdOut); int failCount = 0; PythonQtTestApi api; @@ -70,9 +76,6 @@ int main( int argc, char **argv ) Py_Finalize(); } - PythonQtTestCleanup cleanup; - failCount += QTest::qExec(&cleanup, argc, argv); - if (failCount) { std::cerr << "Tests failed: " << failCount << std::endl; } else { From 83fb76d5415e3b22092a1230c2310ab661c36ae4 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Fillion-Robin Date: Tue, 2 Sep 2025 01:55:11 -0400 Subject: [PATCH 10/14] fix: gate asyncio import during init; disable it in cleanup CI to avoid early ssl load PythonQt::init() unconditionally imported `asyncio`. On Python 3.12 this pulls in `asyncio.sslproto` -> `ssl` -> the `_ssl` C extension. In our "cleanup" test on Ubuntu 24.04 (with sanitizers and rapid finalize), that early `_ssl` load segfaulted inside import/teardown. --- .github/workflows/build.yml | 1 + src/PythonQt.cpp | 17 +++++++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5ced57f91..e07180be7 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -88,6 +88,7 @@ jobs: 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 diff --git a/src/PythonQt.cpp b/src/PythonQt.cpp index 387f7d508..cfaad5cdd 100644 --- a/src/PythonQt.cpp +++ b/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 From fc5554e680f3f3fb1f9034a56393ddddd47dd82e Mon Sep 17 00:00:00 2001 From: Jean-Christophe Fillion-Robin Date: Wed, 3 Sep 2025 22:07:04 -0400 Subject: [PATCH 11/14] fix: Ensure proper cleanup in PythonQtTestCleanup This changes the order of cleanup operations in PythonQtTestCleanup::cleanup(). By calling PythonQt::cleanup() before checking if Python is initialized, we ensure the proper finalization of resources. --- tests/PythonQtTestCleanup.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/PythonQtTestCleanup.cpp b/tests/PythonQtTestCleanup.cpp index a88b66c3b..70d5d427c 100644 --- a/tests/PythonQtTestCleanup.cpp +++ b/tests/PythonQtTestCleanup.cpp @@ -24,14 +24,13 @@ void PythonQtTestCleanup::init() void PythonQtTestCleanup::cleanup() { - // Finalize and cleanup after each test + // Cleanup PythonQt resources before finalizing Python + PythonQt::cleanup(); if (Py_IsInitialized()) { Py_Finalize(); } - PythonQt::cleanup(); - delete _helper; _helper = nullptr; } From f5391ab1b4a3a367fa0014f74c29fd395997c6fa Mon Sep 17 00:00:00 2001 From: Jean-Christophe Fillion-Robin Date: Wed, 3 Sep 2025 22:20:10 -0400 Subject: [PATCH 12/14] tests(PythonQtTestCleanup): Fix unknown attribute in testCallQtMethodInDel This change ensures that the `QTimer` object is properly deleted to avoid the AttributeError when the `__del__` method is called. Fixes the following error: ``` Exception ignored in: Traceback (most recent call last): File "", line 6, in __del__ AttributeError: 'QTimer' object has no attribute 'setSingleShot' ``` --- tests/PythonQtTestCleanup.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/PythonQtTestCleanup.cpp b/tests/PythonQtTestCleanup.cpp index 70d5d427c..e7d3ee8e7 100644 --- a/tests/PythonQtTestCleanup.cpp +++ b/tests/PythonQtTestCleanup.cpp @@ -54,6 +54,7 @@ void PythonQtTestCleanup::testCallQtMethodInDel() " def __del__(self):\n" \ " self.timer.setSingleShot(True)\n" \ "x = TimerWrapper()\n" \ + "del x\n" \ "obj.setPassed()\n" )); } From 8871ee98e578119327430528226399ea2f87611a Mon Sep 17 00:00:00 2001 From: Jean-Christophe Fillion-Robin Date: Tue, 2 Sep 2025 15:41:42 -0400 Subject: [PATCH 13/14] ci(tests): suppress CPython 3.12 interned-unicode leak (PyUnicode_New) in sanitizer runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under ASan/LSan with Python 3.12 on Ubuntu 22.04/24.04, the cleanup tests report direct leaks rooted in CPython’s unicode interning path (frames via PyUnicode_New) during module import in our embedded init. This is a known CPython issue (python/cpython#113190). Interned-unicode cleanup was re-enabled in 3.13 (python/cpython#113601) but not backported to 3.12. To keep CI signal meaningful while we still test against 3.12, enable a narrow LSan suppression for PyUnicode_New only, and only on 3.12. Notes: - detect_leaks=1 remains enabled; the suppression is limited to PyUnicode_New and won’t mask leaks in PythonQt. - Remove once CI moves off Python 3.12. Refs: python/cpython#113190, python/cpython#113601 --- .github/workflows/build.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e07180be7..10dc3d84e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -85,6 +85,14 @@ jobs: - 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 \ From 9e7c519c5093f843534f707a3a2dd9454100403d Mon Sep 17 00:00:00 2001 From: Jean-Christophe Fillion-Robin Date: Wed, 3 Sep 2025 22:39:06 -0400 Subject: [PATCH 14/14] tests(cleanup): clarify destructor tests and add weakref-guarded variant - Rename `testCallQtMethodInDel` to make intent explicit. - Add `testCallQtMethodInDestructorWeakRefGuarded` to cover the pattern where `__del__` dereferences a `weakref` before calling a Qt method. --- tests/PythonQtTestCleanup.cpp | 18 +++++++++++++++++- tests/PythonQtTestCleanup.h | 3 ++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/PythonQtTestCleanup.cpp b/tests/PythonQtTestCleanup.cpp index e7d3ee8e7..d8d652fa2 100644 --- a/tests/PythonQtTestCleanup.cpp +++ b/tests/PythonQtTestCleanup.cpp @@ -44,7 +44,7 @@ void PythonQtTestCleanup::testQtEnum() )); } -void PythonQtTestCleanup::testCallQtMethodInDel() +void PythonQtTestCleanup::testCallQtMethodInDestructorOwnedQTimer() { QVERIFY(_helper->runScript( "import PythonQt.QtCore\n" \ @@ -59,6 +59,22 @@ void PythonQtTestCleanup::testCallQtMethodInDel() )); } +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(); diff --git a/tests/PythonQtTestCleanup.h b/tests/PythonQtTestCleanup.h index a7c73b665..82b14d547 100644 --- a/tests/PythonQtTestCleanup.h +++ b/tests/PythonQtTestCleanup.h @@ -17,7 +17,8 @@ private Q_SLOTS: void cleanup(); void testQtEnum(); - void testCallQtMethodInDel(); + void testCallQtMethodInDestructorOwnedQTimer(); + void testCallQtMethodInDestructorWeakRefGuarded(); void testSignalReceiverCleanup(); private: