Fix crashes during and after PythonQt cleanup #292
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jcfr
force-pushed
the
backport-commontk/fix-cleanup
branch
from
788184b to
83f67ae
Compare
usiems
reviewed
Sep 1, 2025
This comment was marked as outdated.
This comment was marked as outdated.
jcfr
force-pushed
the
backport-commontk/fix-cleanup
branch
2 times, most recently
from
f9bef68 to
e1f8ab4
Compare
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@bf07cc8) Co-authored-by: Jean-Christophe Fillion-Robin <[email protected]>
jcfr
force-pushed
the
backport-commontk/fix-cleanup
branch
from
e1f8ab4 to
ba6c478
Compare
Hm, given that the sanitizer test fails in Ubuntu 24.04, it may be needed (given that I understood the meaning)... though it would be preferable to understand/fix the problem. |
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@4ad7e74) Co-authored-by: Jean-Christophe Fillion-Robin <[email protected]> Co-authored-by: Pat Marion <[email protected]>
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@f0ea5bf)
This is a follow-up to HEAD~3 ("Prevent crashes during and after cleanup")
(cherry picked from commit commontk/PythonQt@8aa183b)
…anup() 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@e4bd663)
Suggested-by: Uwe Siems <[email protected]>
…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
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.
…id 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.
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.
…InDel 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: <function TimerWrapper.__del__ at 0x510000073760> Traceback (most recent call last): File "<string>", line 6, in __del__ AttributeError: 'QTimer' object has no attribute 'setSingleShot' ```
…) in sanitizer runs 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
- Rename `testCallQtMethodInDel` to make intent explicit. - Add `testCallQtMethodInDestructorWeakRefGuarded` to cover the pattern where `__del__` dereferences a `weakref` before calling a Qt method.
jcfr
force-pushed
the
backport-commontk/fix-cleanup
branch
from
ba6c478 to
9e7c519
Compare
mrbean-bremen
approved these changes
Sep 4, 2025
Comment on lines
+88
to
+90
| # 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. |
There was a problem hiding this comment.
Ok, so this is a known problem - thanks for investigating this!
There was a problem hiding this comment.
Indeed, this has been fixed in Python 3.13 but no backport for 3.12 hence the need for the suppression.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This set of patches addresses various crashes occurring during and after the
cleanup of PythonQt.
The changes include:
PythonQtSignalReceiverduring cleanup by removing signal handlers in advance.PythonQtInstanceWrapper_deallochandles the case where PythonQt has been cleaned up.These changes improve the robustness and reliability of PythonQt cleanup processes.
Note
For reference, those patches were developed in the context of the
commontk/PythonQtfork.Cherry picked from commits: