Fix the PySide wrapper-recycling GUI test flake#26
Merged
Conversation
…g flake
The intermittent CI failure ("'QGraphicsItemGroup' object has no attribute
'connect'/'triggered'" inside QMenu.addAction, 5 hits today across PR and
main runs, always in test_gui_processing_panel.py) is Shiboken wrapper
recycling: QGraphicsItem is not a QObject, so when a canvas's scene is
destroyed C++-side, Shiboken never invalidates the Python wrappers for its
items. A wrapper that outlives its C++ item keeps a dangling pointer binding
in Shiboken's cache, and when a later C++ allocation (the QAction QMenu
creates) reuses that heap address, the stale wrapper is resurrected as the
wrong type.
Two layers:
- ImageCanvas connects its destroyed signal to a hook that drops every
scene-item reference (_roi_items, markers, selection/preview/pixmap/
overlay items). QObject.destroyed fires before children are deleted, so
the item wrappers deallocate — unregistering their bindings — while the
C++ items are still alive. The closure captures the attribute dict, not
self, so it cannot keep the view alive. This holds even in the hazardous
case where the canvas *wrapper* is still referenced from Python while its
C++ object dies (a deleteLater()'d dialog still held somewhere).
- The conftest GUI-test drain gains a second gc.collect AFTER deferred
deletions: step 4 destroys C++ objects whose wrappers were not garbage at
the first collect; Shiboken invalidates those QObject wrappers but their
__dict__ (often holding item wrappers) survived until a nondeterministic
collection inside a LATER test — exactly while that test allocates new Qt
objects onto recycled addresses.
tests/test_canvas_item_lifetime.py pins the mechanism: C++ destruction with
the canvas wrapper still referenced must release every item wrapper (fails
without the hook), the hook must not leak the canvas, and plain Python
teardown is unaffected. The flake never reproduced locally, so CI over the
coming runs is the real verdict; the regression test guards the mechanism
either way.
Co-Authored-By: Claude Fable 5 <[email protected]>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Root-causes and fixes the intermittent
'QGraphicsItemGroup' object has no attribute 'connect'/'triggered'failure (5 CI hits today, alwaystest_gui_processing_panel.py).Mechanism:
QGraphicsItemis not a QObject — when a scene dies C++-side, Shiboken never invalidates its items' Python wrappers. A wrapper outliving its C++ item leaves a dangling pointer binding in Shiboken's cache; a later C++ allocation (theQActionthatQMenu.addActioncreates) reusing that address resurrects the stale wrapper as the wrong type.Fix, two layers:
ImageCanvas.destroyedhook drops every scene-item reference before children are deleted (destroyed fires first), so item wrappers unregister their bindings while the C++ items are still alive — even when the canvas wrapper itself is still referenced (adeleteLater()'d dialog still held somewhere).gc.collect()after deferred deletions, collecting wrappers that were invalidated (not garbage) at the first collect — previously they were freed at a nondeterministic point inside a later test, exactly while it allocated onto recycled addresses.Test plan
tests/test_canvas_item_lifetime.py: the key test fails without the hook (verified by stash), passes with it; hook doesn't leak the canvas; plain teardown unaffected.🤖 Generated with Claude Code