From 10ba5b4afdeb452a390d2162f7cfafda0c34f762 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Sat, 10 Oct 2026 15:09:24 -0400 Subject: [PATCH 1/2] Drop a pending flash registration when Qt destroys its widget The flash coordinator held registrations for widgets outside a flash window (QMainWindow/QDialog) for the life of the process and called widget.isVisible()/window() on every local flash, relying on RuntimeError for deleted widgets. A wrapper of a Qt-created widget, such as a scroll area's viewport reached through parentWidget(), is not always marked deleted by sip, so that call read freed memory and segfaulted. Each pending registration now ends when Qt destroys its widget or any widget its element closes over; the RuntimeError probe is gone. 0.3.30. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 9 ++ src/pyqt_reactive/__init__.py | 2 +- src/pyqt_reactive/animation/flash_mixin.py | 132 +++++++++++++-------- tests/test_flash_destruction_lifetime.py | 46 ++++++- 4 files changed, 134 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d123f7a..21f0d43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## 0.3.30 + +- A flash registration waiting for its widget to reach a flash window is + dropped when Qt destroys any widget it closes over. Before, the coordinator + kept such registrations for the life of the process and called into their + widgets on every local flash; a wrapper of a Qt-created widget (such as a + scroll area's viewport) is not marked deleted by sip, so that call read + freed memory and segfaulted. + ## 0.3.29 - Accept zmqruntime 0.5 (`zmqruntime>=0.3.0,<0.6`). zmqruntime 0.5 deletes diff --git a/src/pyqt_reactive/__init__.py b/src/pyqt_reactive/__init__.py index b0c1b6b..9872f71 100644 --- a/src/pyqt_reactive/__init__.py +++ b/src/pyqt_reactive/__init__.py @@ -19,7 +19,7 @@ - Cross-window reactive updates """ -__version__ = "0.3.29" +__version__ = "0.3.30" # Public API will be populated as modules are added __all__ = [ diff --git a/src/pyqt_reactive/animation/flash_mixin.py b/src/pyqt_reactive/animation/flash_mixin.py index dacc370..ba9d86c 100644 --- a/src/pyqt_reactive/animation/flash_mixin.py +++ b/src/pyqt_reactive/animation/flash_mixin.py @@ -2359,6 +2359,16 @@ def paintEvent(self, event) -> None: )) +@dataclass(frozen=True, eq=False) +class _PendingRegistration: + """A flash element waiting for its widget to reach a flash window.""" + + key: str + element_factory: Callable[[], Optional['FlashElement']] + widget: QWidget + source_id: str | None + + # ==================== GLOBAL ANIMATION COORDINATOR ==================== # Single timer shared across ALL windows - batch computes colors, triggers repaints @@ -2434,7 +2444,7 @@ def __init__(self, application: Optional[QCoreApplication] = None): self._pending_visual_frame_callbacks: Dict[int, Callable[[], None]] = {} self._visual_frame_callback_flush_scheduled = False # Pending registrations (widgets not in window hierarchy at registration time) - self._pending_registrations: List[Tuple[str, Callable[[], Optional['FlashElement']], QWidget, str | None]] = [] + self._pending_registrations: list[_PendingRegistration] = [] self._trace_next_tick = False self.start_timer_requested.connect( self._start_timer_in_owner_thread, @@ -2616,9 +2626,27 @@ def add_pending_registration( element_factory: Callable[[], Optional['FlashElement']], widget: QWidget, source_id: str | None = None, + lifecycle_widgets: tuple[QWidget | None, ...] = (), ) -> None: - """Add a pending registration (widget not in window hierarchy yet).""" - self._pending_registrations.append((key, element_factory, widget, source_id)) + """Hold a registration until ``widget`` reaches a flash window. + + The registration lives exactly as long as every widget it closes over: + Qt destruction of any of them drops it. A wrapper of a Qt-created child + (for example a scroll area's viewport) is never marked deleted by sip, + so the registration must not outlive the native widget and touch it. + """ + registration = _PendingRegistration(key, element_factory, widget, source_id) + self._pending_registrations.append(registration) + lifetime_widgets = { + id(candidate): candidate + for candidate in (widget, *lifecycle_widgets) + if candidate is not None + } + for lifetime_widget in lifetime_widgets.values(): + _DestructionCallback( + lifetime_widget, + partial(self._drop_pending_registration, registration), + ) flash_trace( "register.pending_add", key=key, @@ -2627,62 +2655,61 @@ def add_pending_registration( pending=len(self._pending_registrations), ) - def _process_pending_registrations(self) -> None: - """Process all deferred element registrations (widgets now in window hierarchy). + def _drop_pending_registration(self, registration: '_PendingRegistration') -> None: + """Forget a pending registration whose widget Qt destroyed.""" - RESILIENT: Automatically discards registrations for deleted widgets. - This handles the case where widgets are deleted and recreated (e.g., function panes). - """ + self._pending_registrations = [ + pending for pending in self._pending_registrations + if pending is not registration + ] + flash_trace("register.pending_drop_deleted", key=registration.key) + + def _process_pending_registrations(self) -> None: + """Attach every pending registration whose widget now has a flash window.""" if not self._pending_registrations: return logger.debug(f"[FLASH] _process_pending_registrations: processing {len(self._pending_registrations)} pending") - still_pending = [] - for key, element_factory, widget, source_id in self._pending_registrations: - try: - # Check if widget is still valid (not deleted) - # Accessing any Qt property will raise RuntimeError if deleted - _ = widget.isVisible() - overlay = WindowFlashOverlay.get_for_window(widget) - if overlay is not None: - if overlay.has_element_source(key, source_id): - flash_trace( - "register.pending_existing_source", - key=key, - overlay=id(overlay), - window=id(overlay._window), - source=source_id, - ) - continue - element = element_factory() - if element is not None: - overlay.register_element(element) - flash_trace( - "register.pending_attached", - key=key, - overlay=id(overlay), - window=id(overlay._window), - source=element.source_id, - ) - logger.debug(f"[FLASH] Completed deferred registration: key={key}") - else: - logger.debug(f"[FLASH] element_factory returned None for key={key}") - else: - logger.debug(f"[FLASH] No overlay for widget, keeping pending: key={key}") - flash_trace( - "register.pending_wait_overlay", - key=key, - widget=type(widget).__qualname__, - ) - still_pending.append((key, element_factory, widget, source_id)) - except RuntimeError: - # Widget was deleted - discard this registration silently - flash_trace("register.pending_drop_deleted", key=key) - logger.debug(f"[FLASH] Discarding registration for deleted widget: key={key}") + settled: set[int] = set() + for registration in tuple(self._pending_registrations): + key = registration.key + source_id = registration.source_id + overlay = WindowFlashOverlay.get_for_window(registration.widget) + if overlay is None: + flash_trace( + "register.pending_wait_overlay", + key=key, + widget=type(registration.widget).__qualname__, + ) continue + settled.add(id(registration)) + if overlay.has_element_source(key, source_id): + flash_trace( + "register.pending_existing_source", + key=key, + overlay=id(overlay), + window=id(overlay._window), + source=source_id, + ) + continue + element = registration.element_factory() + if element is None: + logger.debug(f"[FLASH] element_factory returned None for key={key}") + continue + overlay.register_element(element) + flash_trace( + "register.pending_attached", + key=key, + overlay=id(overlay), + window=id(overlay._window), + source=element.source_id, + ) - logger.debug(f"[FLASH] _process_pending_registrations: {len(still_pending)} still pending") - self._pending_registrations = still_pending + self._pending_registrations = [ + pending for pending in self._pending_registrations + if id(pending) not in settled + ] + logger.debug(f"[FLASH] _process_pending_registrations: {len(self._pending_registrations)} still pending") def process_pending_registrations(self) -> None: """Public method to process pending registrations. @@ -3432,6 +3459,7 @@ def _register_flash_element_internal( lambda: element_factory(scoped_key), widget, source_id, + lifecycle_widgets, ) flash_trace( "register.deferred_no_overlay", diff --git a/tests/test_flash_destruction_lifetime.py b/tests/test_flash_destruction_lifetime.py index 79733d7..d5208e9 100644 --- a/tests/test_flash_destruction_lifetime.py +++ b/tests/test_flash_destruction_lifetime.py @@ -5,9 +5,13 @@ from PyQt6 import sip from PyQt6.QtCore import QCoreApplication, QEvent -from PyQt6.QtWidgets import QDialog, QTreeWidget, QTreeWidgetItem, QWidget +from PyQt6.QtWidgets import QDialog, QScrollArea, QTreeWidget, QTreeWidgetItem, QWidget -from pyqt_reactive.animation.flash_mixin import VisualUpdateMixin, WindowFlashOverlay +from pyqt_reactive.animation.flash_mixin import ( + VisualUpdateMixin, + WindowFlashOverlay, + _GlobalFlashCoordinator, +) class FlashOwner(QWidget, VisualUpdateMixin): @@ -38,3 +42,41 @@ def queue_window_destruction(): qapp.processEvents() window = window_ref() assert window is None or sip.isdeleted(window) + + + +def _pending_widgets(): + return [ + registration.widget + for registration in _GlobalFlashCoordinator.get()._pending_registrations + ] + + +def test_pending_registration_ends_with_every_widget_it_closes_over(qapp): + """A registration waiting for a flash window is dropped when Qt destroys + any widget it closes over, so it never calls into a freed widget. + + The scroll area's viewport is created by Qt, not Python: its wrapper can + outlive the native widget without sip marking it deleted. + """ + + host = QWidget() # not a QMainWindow/QDialog: registrations must wait + owner = FlashOwner(host) + area = QScrollArea(host) + viewport = area.viewport() + leaf_host = QWidget(host) + leaf = QWidget(leaf_host) + owner.register_flash_widget_rect("viewport", viewport) + owner.register_flash_leaf("leaf", host, leaf) + + assert _pending_widgets().count(viewport) == 1 + assert _pending_widgets().count(host) == 1 + + area.deleteLater() + leaf_host.deleteLater() + QCoreApplication.sendPostedEvents(None, QEvent.Type.DeferredDelete) + + assert viewport not in _pending_widgets() + assert host not in _pending_widgets() + host.deleteLater() + QCoreApplication.sendPostedEvents(None, QEvent.Type.DeferredDelete) From 02849743c2f3c9c9f3cbd4859f1b441a249ab432 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Sat, 10 Oct 2026 15:33:53 -0400 Subject: [PATCH 2/2] Wrap the pending-registration debug line to the 100-column limit Co-Authored-By: Claude Opus 5.5 --- src/pyqt_reactive/animation/flash_mixin.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/pyqt_reactive/animation/flash_mixin.py b/src/pyqt_reactive/animation/flash_mixin.py index ba9d86c..9005983 100644 --- a/src/pyqt_reactive/animation/flash_mixin.py +++ b/src/pyqt_reactive/animation/flash_mixin.py @@ -2709,7 +2709,10 @@ def _process_pending_registrations(self) -> None: pending for pending in self._pending_registrations if id(pending) not in settled ] - logger.debug(f"[FLASH] _process_pending_registrations: {len(self._pending_registrations)} still pending") + logger.debug( + "[FLASH] _process_pending_registrations: %d still pending", + len(self._pending_registrations), + ) def process_pending_registrations(self) -> None: """Public method to process pending registrations.