fix(ui): prevent segfault during init — call _vt_append via Qt signal from background thread - #1
Conversation
background thread PK232-ParamUpload thread was calling _vt_append() (QTextCursor ops) directly — undefined behaviour in PyQt6, causes segfault at end of params upload. Add _vt_append_signal(str,str) connected to _vt_append in the GUI thread; background thread emits the signal instead.
oe3gas
left a comment
There was a problem hiding this comment.
Thanks for the report and the fix — the diagnosis is exactly right, and the reproduction with real hardware is much appreciated. I'd never seen this on Windows, but that was timing luck rather than a working code path: the uploader idles ≥120 ms between commands, so the GUI thread rarely collides with it. The bug was always there, just latent. Good catch.
The signal/queued-connection approach is the right one, and keeping the default colour in the _vt() wrapper was the correct call — without it the single-argument uses would have raised a TypeError silently inside a daemon thread.
Two things before I can merge — see the inline comments. Short version: _log_monitor() is still called directly from the worker thread and has the same problem, and the two setFocus() calls were dropped rather than moved.
One suggestion, entirely optional: instead of adding a second bridge signal, it might be cleaner to move the whole post-upload tail into the GUI thread — something like _upload_done_signal = pyqtSignal(int, bool) emitted at the end of _upload(), with a slot that handles setFocus() and enter_host_mode(). That leaves the worker thread doing nothing but blocking serial I/O and no UI knowledge at all. Happy either way — the two-signal version is fine if you prefer the smaller diff.
I'll also add the GUI-thread rule to CLAUDE.md §3 and list the PK232-ParamUpload thread in the state-machine doc, where it's currently missing. That omission is arguably why this slipped through in the first place.
Once the two points above are addressed I'll test on Windows (COM16, PK-232MBX v7.1) — including the fast-init branch, which has its own code path.
| if fast_init: | ||
| self._vt_append("[SYS] Fast Init — parameter upload skipped\n") | ||
| _vt("[SYS] Fast Init — parameter upload skipped\n") | ||
| self._log_monitor("[SYS] Fast Init active — no parameter upload") |
There was a problem hiding this comment.
_log_monitor() has the same threading problem as _vt_append() — it writes to the self._monitor QTextEdit and reads self._mon_btn_hex.isChecked(), both GUI-thread-only. Since it's still called directly from _upload(), the segfault isn't fully eliminated, just made much rarer (2 calls instead of 61). Could you route it through a signal as well? Something like _log_monitor_signal = pyqtSignal(str) connected to _log_monitor, following the same pattern you used for _vt_append. Same applies to line 955.
| else: | ||
| self._vt_append("[SYS] Verbose terminal ready (fast init)\n") | ||
| self._vt_input.setFocus() | ||
| _vt("[SYS] Verbose terminal ready (fast init)\n") |
There was a problem hiding this comment.
The self._vt_input.setFocus() call was removed here and at the end of the upload path, but isn't mentioned in the PR description. I understand why — it's a GUI call from a background thread and had to go. But dropping it is a silent behaviour change: after the upload completes, the cursor no longer returns to the command input, which is noticeable if the user clicked elsewhere during the ~7 s upload. Could it be restored via the GUI thread instead of removed?
Problem
Every connect/init sequence ends in a segfault:
INFO pk232py.comm.params_uploader ParamsUploader: upload complete (59 commands)
[1] 464793 segmentation fault (core dumped) pk232py
Root Cause
_on_verbose_mode_readyspawns aPK232-ParamUploadbackground threadthat calls
_vt_append()directly — including via theecho_callbackpassed to
ParamsUploader._vt_appendmanipulates Qt widgets(
QTextCursor,insertText,ensureCursorVisible), which must onlybe called from the GUI thread. Doing so from a background thread is
undefined behaviour in PyQt6 and causes the segfault.
Fix
_vt_append_signal = pyqtSignal(str, str)toMainWindow_vt_appendin_connect_signals()(queued → GUI thread)_vt_append()calls inside the_upload()closurewith a local
_vt()wrapper that emits the signal instead_vtasecho_callbacktoParamsUploaderTesting
Reproduced segfault on Linux with a real PK-232MBX (v7.1). After this
fix the full init sequence completes without crash and the verbose
terminal displays all uploaded parameters correctly.