From bbf3657ce7ffdb14ca62dbcf0cc84044f03657fd Mon Sep 17 00:00:00 2001 From: Loren Eteval Date: Mon, 24 Aug 2026 18:41:41 +0800 Subject: [PATCH] Document packaged Qt lifetime verification Extend the Qt lifetime skill with packaged-build ownership hazards, native destruction probes, binding-specific diagnostics, and review guidance for distinguishing real regressions from unavailable protected counters. Signed-off-by: Loren Eteval --- .../manage-qt-pyside6-lifetimes/SKILL.md | 56 ++++++- .../qt-pyside6-object-lifetime-guidelines.md | 148 +++++++++++++++++- 2 files changed, 196 insertions(+), 8 deletions(-) diff --git a/.agents/skills/manage-qt-pyside6-lifetimes/SKILL.md b/.agents/skills/manage-qt-pyside6-lifetimes/SKILL.md index 3d11960..f7dad96 100644 --- a/.agents/skills/manage-qt-pyside6-lifetimes/SKILL.md +++ b/.agents/skills/manage-qt-pyside6-lifetimes/SKILL.md @@ -1,6 +1,6 @@ --- name: manage-qt-pyside6-lifetimes -description: Audit and implement safe Qt/PySide6 object ownership and destruction in Furious. Use whenever work creates, modifies, reviews, or diagnoses QObject, QWidget, QDialog, QAction, QMenu, QTimer, models, delegates, animations, event filters, signals/slots, UI registries, transient or reusable windows, Qt-related caches, memory growth, premature window disappearance, stale wrappers, or native-versus-packaged lifetime differences. +description: Audit and implement safe Qt/PySide6 object ownership and destruction in Furious. Use for QObject/UI lifetimes, transient or reusable windows, signals/slots, direct bound-method connections, weak dispatch, delete-on-close dialogs, timers, models, delegates, registries, stale wrappers, memory growth, premature destruction, and native-versus-Nuitka packaged differences. --- # Manage Qt/PySide6 Lifetimes @@ -23,6 +23,7 @@ For every affected Qt object, record: - close/hide/destroy path; - timers, event filters, models, delegates, menus, actions, animations, and graphics effects it owns; - connections to application-lifetime senders; +- for each relevant signal: sender/receiver lifetimes and QObject trees, direct bound method versus weak dispatcher versus closure/partial, and its disconnect boundary; - caches, registries, closures, partials, lambdas, or callbacks that can retain it. Do not treat `.show()`, `.open()`, `.close()`, a parent, or a weak reference as proof that the lifetime is correct. @@ -61,7 +62,43 @@ Search the affected call paths for: Prefer immutable metadata and classes/factories in caches and registries. Never cache a transient Qt instance or an instance method whose key contains `self`. -### 5. Diagnose before fixing +### 5. Enforce packaged signal safety + +Nuitka's PySide6 integration can retain compiled bound methods passed directly to +`SignalInstance.connect()` in a process-global protection list. A connection that is +harmless under native CPython can therefore retain a transient receiver and its Qt +subtree for the life of the packaged process. + +- Do not connect a signal directly to a bound method of a transient or repeatedly + created `QObject`. +- Use `Furious.Qt.connectWeakly(signal, receiver, 'methodName', ...)`. +- Pass `sender=` when the sender is independently owned or longer-lived so the helper + can remove the dormant dispatcher when the receiver is destroyed. Use + `forwardSender=True` instead of relying on `QObject.sender()` when the slot needs the + sender. +- Do not substitute a lambda or partial that strongly captures the receiver. +- Direct bound-method connections are acceptable only for deliberately process-lifetime + receivers when the resulting strong retention is intentional and documented. + +The weak dispatcher must keep only weak receiver/sender references, check PySide wrapper +validity, and resolve the method by name at emission time. + +### 6. Preserve asynchronous dialog destruction + +For non-blocking dialogs, distinguish interaction completion from native destruction: + +- reusable dialogs may release an open-dialog registry entry at `finished`; +- one-shot dialogs using `WA_DeleteOnClose` must remain strongly retained after + `finished`, through deferred Qt deletion, until `destroyed` has been dispatched; +- release the registry entry on the next event-loop turn after `destroyed`; +- registry callbacks must capture an opaque lifetime token, not the dialog; +- operation-specific context may be released at `finished` once callbacks no longer + need it. + +Use the existing `AppQDialog`/`AppQTransientDialog`/`AppQMessageBox` ownership model +rather than adding a parallel registry. + +### 7. Diagnose before fixing Use targeted evidence as needed: @@ -74,7 +111,7 @@ Use targeted evidence as needed: Distinguish retained objects from Python allocator high-water marks and Qt/native memory caching. Remove temporary diagnostics after the cause is understood. -### 6. Verify the lifecycle +### 8. Verify the lifecycle Run the narrow behavior test first, then the applicable Qt lifetime tier in `tests/README.md`. For shared transient infrastructure, repeat at least 20-50 cycles and verify: @@ -85,6 +122,15 @@ Run the narrow behavior test first, then the applicable Qt lifetime tier in `tes - asynchronous windows retain a Python owner while visible; - native Python remains correct and the packaged build is checked when the issue is packaging-specific. +For transient signal/dialog infrastructure, run both the native lifecycle tests and a +Nuitka-compiled repeated-open/close probe. Cover `accept`, `reject`, and window-close +paths plus representative protocol/editor mixes. Assert that destroyed counts match, +weak wrappers and registries return to zero, operation context is released, no invalid +wrapper is accessed, and Nuitka's protected callback collection does not grow when that +internal diagnostic is observable. If it is hidden by the compiled runtime, combine +zero retained wrappers with inspection of the selected Nuitka package configuration; +do not report an unobservable counter as measured. + ## Prohibited shortcuts Do not use routine `gc.collect()`, global retention of every window, indiscriminate `WA_DeleteOnClose`, hiding instead of destroying, broad deleted-wrapper exception suppression, or ever-growing thresholds as standalone fixes. @@ -98,4 +144,6 @@ Before handing off a Qt-related change, be able to explain: 3. the exact destruction or reuse path; 4. why signals, timers, filters, caches, and registries cannot retain stale UI; 5. why the object cannot disappear prematurely; -6. which repeated lifecycle verification passed. +6. whether direct signal callbacks or closures create unwanted packaged-build retention; +7. for delete-on-close dialogs, why the final owner survives until native destruction; +8. which native and, when relevant, Nuitka-compiled lifecycle verification passed. diff --git a/.agents/skills/manage-qt-pyside6-lifetimes/references/qt-pyside6-object-lifetime-guidelines.md b/.agents/skills/manage-qt-pyside6-lifetimes/references/qt-pyside6-object-lifetime-guidelines.md index fd6c0e9..782b3b8 100644 --- a/.agents/skills/manage-qt-pyside6-lifetimes/references/qt-pyside6-object-lifetime-guidelines.md +++ b/.agents/skills/manage-qt-pyside6-lifetimes/references/qt-pyside6-object-lifetime-guidelines.md @@ -34,7 +34,15 @@ Treat memory leaks, stale object retention, dangling Qt wrappers, and premature ## 1. Core Lifetime Principle -Every `QObject`-derived object should have an intentional owner, lifetime, and destruction strategy. +Every `QObject`-derived object should have an intentional owner, lifetime, and destruction strategy. Every signal/callback edge that can extend that lifetime must also have an intentional retention and cleanup strategy. + +A packaged PySide6 feature can involve three overlapping lifetime systems: + +1. Python wrappers, callables, closures, and reference ownership; +2. Qt/C++ parent ownership, signal dispatch, deferred deletion, and native destruction; +3. compiler/runtime compatibility retention, including Nuitka's protection of selected compiled callbacks. + +Correct parentage in one system does not prove that the other two match the intended logical lifetime. For each dynamically created object, determine which category it belongs to. @@ -210,6 +218,68 @@ Use Qt's automatic `QObject` disconnection where sufficient. When it is not suff Avoid unnecessary manual disconnect boilerplate when Qt already manages the connection safely. +### Nuitka/PySide6 compiled bound-method retention + +Native PySide6 and a Nuitka-compiled application do not necessarily have the same +Python-callable retention graph. In the currently verified toolchain (Nuitka 4.1.3, +PySide6 6.8.3), Nuitka's standard PySide6 package configuration patches +`SignalInstance.connect()` and `QTimer.singleShot()`. When the callback is a compiled +bound method, the generated post-import code protects it in a process-global list named +`_protected` and may also expose its underlying function on the receiver class. The +compiled runtime does not necessarily publish that list as a `PySide6` module +attribute. This protection keeps the bound receiver strongly reachable. Repeated +transient receivers can therefore grow for the whole packaged-process lifetime even +when native CPython destroys them. + +The same protection pattern exists in current upstream Nuitka source. Related PySide6 +workaround behavior is documented for earlier Nuitka/PySide6 combinations, but do not +assume an exact introduction version without checking the selected release. Always +inspect the package configuration installed in the environment being shipped. + +The following is prohibited for a transient or repeatedly created receiver: + +```python +sender.signal.connect(transientReceiver.handleSignal) +QTimer.singleShot(0, transientReceiver.finishWork) +``` + +Replacing the slot with a lambda or `partial` is not safe if it strongly captures the +receiver: + +```python +sender.signal.connect(lambda: transientReceiver.handleSignal()) +``` + +In Furious, use the canonical weak dispatcher: + +```python +connectWeakly( + sender.signal, + transientReceiver, + 'handleSignal', + sender=sender, +) +``` + +`connectWeakly()` has this contract: + +- the connected dispatcher is a plain function, not the receiver's bound method; +- receiver and optional sender are stored only through `weakref.ref`; +- the method is resolved by its string name only when the signal is emitted; +- `shiboken6.isValid()` is checked before accessing a `QObject` wrapper; +- `forwardSender=True` explicitly passes the sender instead of depending on + `QObject.sender()`; +- when the sender is independently owned or longer-lived, `sender=` lets receiver + destruction disconnect the otherwise dormant dispatcher; +- the method name is static and must remain valid for the receiver's lifetime. + +Pass the sender whenever it is not in the receiver's QObject subtree. Omitting it can +leave safe no-op dispatchers attached to a long-lived sender even though the weak +receiver itself is gone. A direct bound-method connection is permitted only when the +receiver is deliberately process-lifetime, the retention is intentional and +documented, and the connection is not repeatedly recreated. Prefer weak dispatch for +dynamic or repeated connections regardless. + ## 9. Timers Every `QTimer` should have an intentional owner and stop policy. @@ -257,10 +327,43 @@ For modal dialogs using `exec()`, local ownership may be sufficient because exec For non-blocking dialogs using `.show()` or `.open()`: - retain a strong reference while visible; -- release it intentionally when destroyed. +- release it at the lifecycle boundary appropriate to the dialog type. Never rely on an unreferenced local variable for an asynchronous dialog. Also verify repeated dialog creation does not cause memory growth. +### `finished` is not native destruction + +For a one-shot dialog using `WA_DeleteOnClose`, `finished` reports that the interaction +ended; it does not prove that Qt has destroyed the native object. Native deletion is +deferred. If an asynchronous registry releases the final Python reference at +`finished`, the wrapper can disappear before Qt completes deletion, or a stale wrapper +can survive after the native object is gone. + +Use this sequence for transient delete-on-close dialogs: + +1. create a unique opaque lifetime token; +2. insert `token -> dialog` into the open-dialog registry before calling `open()`; +3. allow `accept`, `reject`, or window close to emit `finished`; +4. keep the strong registry entry while `WA_DeleteOnClose` schedules native deletion; +5. observe `destroyed`; +6. remove the token on the next event-loop turn. + +Callbacks that schedule registry cleanup must capture only the opaque token, never the +dialog. An identifier derived from `id(dialog)` is weaker because object IDs can be +reused. Operation-specific context may be released at `finished` after its callbacks +run, provided the lifetime registry still retains the dialog itself through +destruction. + +Reusable dialogs follow a different policy. A reusable dialog is normally hidden at +`finished`, not deleted, so its temporary open-dialog registry entry may be released at +`finished` while its deliberate owner continues to retain it. Do not add +`WA_DeleteOnClose` merely to make cleanup uniform. + +Furious implements these policies through `AppQDialog`, `AppQTransientDialog`, and +`AppQMessageBox`. `AppQMessageBox` currently has an additional registry because its +QMessageBox-compatible `open()` path bypasses the base implementation; its cleanup must +remain synchronized with the base dialog policy or be deliberately consolidated. + ## 14. Packaged and Compiled Builds Require Extra Caution Object lifetime behavior that appears acceptable under native Python can expose problems more clearly in packaged or compiled builds. @@ -274,6 +377,15 @@ Do not assume operating-system task-manager memory alone proves a leak. Distingu The strongest evidence of a real leak is continued growth in live object/resource counts after repeated create/close cycles. +For Nuitka/PySide6 signal-retention work, include a compiled diagnostic that records +the size of Nuitka's `_protected` callback list before and after repeated cycles when +the compiled runtime exposes that internal diagnostic. It is not a production API and +may be hidden even though the post-import protection is active. Combine it with +`QObject.destroyed`, weak references, open-dialog registry size, operation-context +counts, and `shiboken6.isValid()` checks. Zero protected-list growth alone is not proof +of correct destruction, a missing counter is not zero growth, and stable process memory +alone is not proof of no leak. + ## 15. Required Lifetime Review for UI Changes Whenever a change creates or modifies dynamically managed Qt objects, explicitly review: @@ -316,8 +428,8 @@ Representative procedure: 1. Record baseline live-object counts. 2. Open the dialog/editor. -3. Close it. -4. Repeat 20–50 times. +3. Exercise `accept`, `reject`, and window-close paths where applicable. +4. Repeat 20–100 times. 5. Verify objects intended to die are destroyed. 6. Verify weak references clear. 7. Verify relevant pools/registries return to baseline. @@ -325,6 +437,20 @@ Representative procedure: For shared editor infrastructure, test multiple editor/dialog types rather than only one. +Run the same representative probe natively and as a standalone Nuitka build when the +code uses PySide6 signals or asynchronous transient dialogs. Vary protocol/editor order +so one family cannot hide a shared-registry or cached-callback defect. Required results +are: + +- every expected `destroyed` signal fires; +- weak wrappers, open-dialog registries, and operation contexts return to zero; +- no wrapper becomes invalid before the close path completes; +- the registry still holds each transient dialog when `finished` is dispatched; +- Nuitka's protected callback collection has zero growth when observable; otherwise, + the compiled probe has zero retained wrappers and the selected package configuration + confirms that the dispatcher is not an eligible protected bound method; +- no per-cycle `gc.collect()` is needed to obtain those results. + ## 18. Do Not Mask Lifetime Bugs The following are not acceptable as standalone fixes: @@ -355,6 +481,18 @@ When reviewing existing code or implementing a refactor, treat lifetime correctn If suspicious ownership is encountered while working on an unrelated feature, investigate and correct it when reasonably within scope. At minimum, do not introduce new lifetime ambiguity. +Reject a change when it: + +- passes a transient or repeatedly created receiver's bound method directly to a + PySide6 signal or `QTimer.singleShot()` in packaged-sensitive code; +- replaces that connection with a lambda/partial that strongly captures the receiver; +- omits `sender=` for `connectWeakly()` when an independently owned or longer-lived + sender needs destroyed-receiver cleanup; +- releases a delete-on-close asynchronous dialog's final strong owner at `finished`; +- captures the transient dialog in a registry-cleanup callback; +- verifies only native CPython when the defect can be introduced by Nuitka's PySide6 + integration. + ## Acceptance Criteria For Qt/PySide6 UI code, a correct implementation should satisfy all applicable conditions: @@ -370,5 +508,7 @@ For Qt/PySide6 UI code, a correct implementation should satisfy all applicable c - No visible window disappears because its wrapper is prematurely garbage-collected. - Native Python execution remains correct. - Packaged/compiled execution remains correct where testable. +- Transient/repeated PySide6 connections do not grow Nuitka's protected bound-method retention. +- Delete-on-close asynchronous dialogs remain retained through `finished` and are released only after `destroyed` dispatch. **Final rule:** Every Qt object in this repository must have an intentional owner, lifetime, and destruction strategy. When in doubt, investigate the lifetime explicitly rather than relying on implicit Python garbage collection or Qt parent behavior.