Skip to content

fix(hud): keep a due message timer off a cleared closure - #1615

Merged
marcinz606 merged 1 commit into
marcinz606:mainfrom
kurtwindisch:fix/hud-timer-crash
Nov 6, 2026
Merged

marcinz606 merged 1 commit into
marcinz606:mainfrom
kurtwindisch:fix/hud-timer-crash

Conversation

@kurtwindisch

@kurtwindisch kurtwindisch commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #1614. make test crashed with a segfault in tests/test_key_hold.py, on main too. The CanvasHud message timer called a lambda whose closure the garbage collector had cleared. The issue has the full cause and a reproduction.

  • CanvasHud: the message timer connects to a bound method, _expire_message, as _progress_timer beside it does. PyQt holds no closure for a bound method, so a collection has nothing to clear.
  • Pick Better: the move timer had the same lambda and connects to QLabel.clear.
  • Regression test: test_a_message_timer_survives_a_collection_on_another_thread runs the reproduction in a subprocess, because the failure is a segfault.

I have no report of this crash in the running app. A HUD normally lives as long as the window, so the known trigger is the test suite. The other lambdas on dlg.finished signals are not timers, and this PR leaves them as they are.

Test plan

  • pytest tests/ --cov=negpy --cov-report=term-missing: crashed in 5 of 5 runs before the change, and passed in 4 of 4 after it (10762 passed)
  • The new test fails on the old hud.py (exit code -11) and passes on the new one
  • make lint and make type pass
  • Drove the app offscreen with a sandboxed user directory: a timed HUD message shows and then clears
  • Pick Better's move summary clears after a pick, in the running app: after one pick, after two picks in a row, and with Esc while the summary shows

🤖 Generated with Claude Code

`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 marcinz606#1614

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kurtwindisch
kurtwindisch marked this pull request as ready for review November 5, 2026 22:36
@marcinz606
marcinz606 merged commit 8eed02b into marcinz606:main Nov 6, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Segfault in test_key_hold: CanvasHud timer lambda runs after the collector clears it

2 participants