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..9005983 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,64 @@ 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( + "[FLASH] _process_pending_registrations: %d still pending", + len(self._pending_registrations), + ) def process_pending_registrations(self) -> None: """Public method to process pending registrations. @@ -3432,6 +3462,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)