From 716755d6f022b705e3254bc1973f1c59f410fa12 Mon Sep 17 00:00:00 2001 From: Kurt Windisch Date: Thu, 5 Nov 2026 13:04:34 -0800 Subject: [PATCH] fix(hud): keep a due message timer off a cleared closure `make test` crashed in test_key_hold.py: the CanvasHud message timer called a lambda whose closure the garbage collector had cleared. test_hud_message_wrap.py leaves a HUD with a pending timer in a reference cycle. A collection on a background thread clears the lambda, and PyQt defers the widget's delete because it is on the wrong thread. The first event loop spin then runs the due timer before the deferred delete, and the lambda reads a NULL closure. The timer now connects to a bound method, as _progress_timer does. Pick Better's move timer had the same lambda and now connects to QLabel.clear. The crashing command failed in 5 of 5 runs before this change and passed in 4 of 4 after it. Fixes #1614 Co-Authored-By: Claude Opus 5.5 --- negpy/desktop/view/canvas/hud.py | 7 +++- .../view/sidebar/pick_better_window.py | 2 +- tests/test_canvas_hud.py | 42 +++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/negpy/desktop/view/canvas/hud.py b/negpy/desktop/view/canvas/hud.py index 171d70460..24b456031 100644 --- a/negpy/desktop/view/canvas/hud.py +++ b/negpy/desktop/view/canvas/hud.py @@ -123,7 +123,9 @@ def __init__(self, parent=None): self._message_timer = QTimer(self) self._message_timer.setSingleShot(True) - self._message_timer.timeout.connect(lambda: self.showMessage("")) + # A bound method, not a lambda: a collection on another thread clears a closure and defers the + # widget's delete, so a due timer would call a function with no closure. + self._message_timer.timeout.connect(self._expire_message) self._progress_timer = QTimer(self) self._progress_timer.setSingleShot(True) self._progress_timer.timeout.connect(self.hide_progress) @@ -160,6 +162,9 @@ def showMessage(self, text: str, timeout: int = 0, kind: str = "info") -> None: self._message_timer.start(timeout or _DEFAULT_MESSAGE_MS) self._sync() + def _expire_message(self) -> None: + self.showMessage("") + def clear_message(self, text: str) -> None: """Clears the message only if it is still `text`, so a later message survives.""" if text and self.message.text() == text and not self.message.isHidden(): diff --git a/negpy/desktop/view/sidebar/pick_better_window.py b/negpy/desktop/view/sidebar/pick_better_window.py index 83a9a8a53..0c4f7987d 100644 --- a/negpy/desktop/view/sidebar/pick_better_window.py +++ b/negpy/desktop/view/sidebar/pick_better_window.py @@ -106,7 +106,7 @@ def __init__(self, parent=None) -> None: self._move_label.setAttribute(Qt.WidgetAttribute.WA_TransparentForMouseEvents) self._move_timer = QTimer(self) self._move_timer.setSingleShot(True) - self._move_timer.timeout.connect(lambda: self._move_label.setText("")) + self._move_timer.timeout.connect(self._move_label.clear) # The photos inset from the panel's top by the reserved band — the move summary, the in-flight # name and the A/Skip/B captions — so a caption that hugs a photo's top edge sits clear of diff --git a/tests/test_canvas_hud.py b/tests/test_canvas_hud.py index 23e3aff23..d19924211 100644 --- a/tests/test_canvas_hud.py +++ b/tests/test_canvas_hud.py @@ -1,3 +1,8 @@ +import os +import subprocess +import sys +from pathlib import Path + from PyQt6.QtGui import QColor from PyQt6.QtWidgets import QWidget @@ -14,6 +19,43 @@ def _hud(width=1200): return hud +_DUE_TIMER_AFTER_A_BACKGROUND_COLLECTION = """ +import gc, threading, time +from PyQt6.QtTest import QTest +from PyQt6.QtWidgets import QApplication, QWidget +from negpy.desktop.view.canvas.hud import CanvasHud + +app = QApplication([]) +gc.disable() + +def strip(): + parent = QWidget() + hud = CanvasHud(parent) + hud.showMessage("x", 20) + hud._keepalive = parent + +strip() +thread = threading.Thread(target=gc.collect) +thread.start() +thread.join() +time.sleep(0.05) +QTest.qWait(30) +""" + + +def test_a_message_timer_survives_a_collection_on_another_thread(): + # In a subprocess: the failure is a segfault, which would end the test run. + run = subprocess.run( + [sys.executable, "-c", _DUE_TIMER_AFTER_A_BACKGROUND_COLLECTION], + capture_output=True, + text=True, + timeout=60, + cwd=Path(__file__).parent.parent, + env={**os.environ, "QT_QPA_PLATFORM": "offscreen"}, + ) + assert run.returncode == 0, run.stderr + + def test_an_empty_strip_hides_so_the_canvas_keeps_its_height(qapp): hud = _hud() assert hud.isHidden()