gui: fix cross-thread widget access that crashed compress (app closes / corrupts)
Reported: files corrupt, compress does nothing on hybrid, app closes on full-PQ compress. Root cause: run_async connected plain Python CLOSURES (finish / on_pct / release) to signals emitted from the worker QThread. PySide6 runs a plain-closure slot in the EMITTING thread regardless of the requested connection type — even an explicit Qt.QueuedConnection — because a bare functor has no receiver QObject to give it GUI-thread affinity (verified: such a slot ran on the worker thread; a bound method of a main-thread QObject ran on MAIN). Those closures then called QProgressBar.setValue/setRange/hide, QPushButton.setEnabled and QTextEdit.append from the worker thread: cross-thread QWidget access is undefined behaviour and crashed the app under real X11/Wayland rendering. It only survived offscreen tests (which tolerate the race), which is why prior driver runs passed. The progress-bar work added in 5.1.0 multiplied the cross-thread calls and made the crash reliable; a crash mid-compress also left truncated/corrupt archives. Fix: a _Job(QObject) controller parented to a GUI-thread widget, so it lives in the GUI thread and every slot (on_log/on_pct/on_done/on_finished) is a bound method that Qt auto-marshals to the GUI thread. closeEvent updated to the new _jobs (list of _Job) shape. Verified on real X (window shown, progress bar rendering) with a QProgressBar/ QPushButton instrumentation that flags ANY worker-thread call: BEFORE = setRange/ setValue/hide flagged on the worker thread; AFTER = zero cross-thread calls, and hybrid + full-PQ + password compress/extract all byte-exact round-trip; Verify/ Info/Disk backup+restore/two-concurrent-jobs/close-mid-job all pass.
This commit is contained in:
parent
7c062bc92c
commit
df534503ec
1 changed files with 81 additions and 43 deletions
|
|
@ -457,6 +457,74 @@ class PathField(QWidget):
|
||||||
def scrollable(w):
|
def scrollable(w):
|
||||||
sa = QScrollArea(); sa.setWidgetResizable(True); sa.setWidget(w); sa.setFrameShape(QFrame.Shape.NoFrame); return sa
|
sa = QScrollArea(); sa.setWidgetResizable(True); sa.setWidget(w); sa.setFrameShape(QFrame.Shape.NoFrame); return sa
|
||||||
|
|
||||||
|
class _Job(QObject):
|
||||||
|
"""Controller for one async CLI run.
|
||||||
|
|
||||||
|
CRITICAL threading contract: this object is parented to a GUI-thread widget,
|
||||||
|
so it LIVES in the GUI thread, and every slot below (on_log/on_pct/on_done/
|
||||||
|
on_finished) is a bound method of a GUI-thread QObject. Qt therefore auto-
|
||||||
|
marshals the worker's signals to the GUI thread (QueuedConnection).
|
||||||
|
|
||||||
|
The previous design connected plain Python CLOSURES (finish/on_pct/release)
|
||||||
|
to signals emitted from the worker thread. PySide6 runs a plain-closure slot
|
||||||
|
in the EMITTING thread regardless of the requested connection type — even an
|
||||||
|
explicit Qt.QueuedConnection — because a bare functor has no receiver QObject
|
||||||
|
to give it thread affinity (verified empirically). Those closures then
|
||||||
|
touched QProgressBar / QPushButton / QTextEdit internals from the worker
|
||||||
|
thread: cross-thread QWidget access, which is undefined behaviour and crashed
|
||||||
|
the app under real X11/Wayland rendering ("the app closes when I compress").
|
||||||
|
It only survived offscreen tests, which tolerate the race. Bound methods of a
|
||||||
|
GUI-thread QObject are the fix."""
|
||||||
|
def __init__(self, parent, cmd, btn, log, progress):
|
||||||
|
super().__init__(parent)
|
||||||
|
self._parent = parent
|
||||||
|
self.btn, self.log, self.progress = btn, log, progress
|
||||||
|
self.thread = QThread(self) # QThread object lives in GUI thread
|
||||||
|
self.worker = Worker(cmd) # no parent — it moves to self.thread
|
||||||
|
self.worker.moveToThread(self.thread)
|
||||||
|
self.worker.log.connect(self.on_log)
|
||||||
|
self.worker.pct.connect(self.on_pct)
|
||||||
|
self.worker.done.connect(self.on_done)
|
||||||
|
self.thread.finished.connect(self.on_finished)
|
||||||
|
self.thread.started.connect(self.worker.run)
|
||||||
|
|
||||||
|
def start(self):
|
||||||
|
self.thread.start()
|
||||||
|
|
||||||
|
def on_log(self, line):
|
||||||
|
self.log.append(line)
|
||||||
|
|
||||||
|
def on_pct(self, p):
|
||||||
|
if self.progress is not None:
|
||||||
|
if self.progress.maximum() != 100:
|
||||||
|
self.progress.setRange(0, 100)
|
||||||
|
self.progress.setValue(p)
|
||||||
|
|
||||||
|
def on_done(self, code, out, err):
|
||||||
|
self.btn.setEnabled(True)
|
||||||
|
if self.progress is not None:
|
||||||
|
self.progress.hide()
|
||||||
|
self.log.append("\nDone." if code == 0 else f"\nFailed (exit {code}).")
|
||||||
|
self.thread.quit()
|
||||||
|
|
||||||
|
def on_finished(self):
|
||||||
|
# Runs on the GUI thread AFTER the QThread has emitted finished(); the
|
||||||
|
# wait() joins the last native teardown so dropping the last Python ref
|
||||||
|
# can't collect a still-running QThread (that aborts with "QThread:
|
||||||
|
# Destroyed while thread is still running").
|
||||||
|
self.thread.wait()
|
||||||
|
try:
|
||||||
|
self._parent._jobs.remove(self)
|
||||||
|
except (AttributeError, ValueError):
|
||||||
|
pass
|
||||||
|
|
||||||
|
def cancel_and_join(self, ms=3000):
|
||||||
|
"""GUI thread: kill the child CLI and join the worker thread."""
|
||||||
|
self.worker.cancel()
|
||||||
|
self.thread.quit()
|
||||||
|
return self.thread.wait(ms)
|
||||||
|
|
||||||
|
|
||||||
def run_async(parent, cmd, btn, log, progress=None, info=None):
|
def run_async(parent, cmd, btn, log, progress=None, info=None):
|
||||||
log.clear()
|
log.clear()
|
||||||
if info: # e.g. an auto-detect note; appended AFTER the clear so it survives
|
if info: # e.g. an auto-detect note; appended AFTER the clear so it survives
|
||||||
|
|
@ -464,46 +532,18 @@ def run_async(parent, cmd, btn, log, progress=None, info=None):
|
||||||
btn.setEnabled(False)
|
btn.setEnabled(False)
|
||||||
if progress:
|
if progress:
|
||||||
progress.setRange(0, 0) # indeterminate until the CLI reports a %
|
progress.setRange(0, 0) # indeterminate until the CLI reports a %
|
||||||
|
progress.setValue(0)
|
||||||
progress.show()
|
progress.show()
|
||||||
t = QThread(); w = Worker(cmd); w.moveToThread(t)
|
# Keep a LIST of live jobs on the parent. Tabs with more than one action
|
||||||
# log.append targets a main-thread QObject -> Qt queues it to the GUI thread.
|
# button (Disk: backup + restore) previously shared a single slot, so
|
||||||
w.log.connect(log.append)
|
# starting a second op dropped the only Python reference to the first
|
||||||
if progress:
|
# still-running QThread and Python GC'd it mid-run. The list holds every
|
||||||
def on_pct(p):
|
# in-flight job (and keeps the QThread alive).
|
||||||
if progress.maximum() != 100:
|
|
||||||
progress.setRange(0, 100)
|
|
||||||
progress.setValue(p)
|
|
||||||
w.pct.connect(on_pct)
|
|
||||||
# Keep a LIST of live (thread, worker) refs on the parent. Tabs with more
|
|
||||||
# than one action button (Disk: backup + restore) previously shared a
|
|
||||||
# single _thread/_worker slot, so starting a second op dropped the only
|
|
||||||
# Python reference to the first still-running QThread — Python GC'd it
|
|
||||||
# mid-run and aborted the operation. A list holds every in-flight thread.
|
|
||||||
if not hasattr(parent, "_jobs"):
|
if not hasattr(parent, "_jobs"):
|
||||||
parent._jobs = []
|
parent._jobs = []
|
||||||
parent._jobs.append((t, w))
|
job = _Job(parent, cmd, btn, log, progress)
|
||||||
def finish(code, out, err):
|
parent._jobs.append(job)
|
||||||
btn.setEnabled(True)
|
job.start()
|
||||||
if progress: progress.hide()
|
|
||||||
log.append("\nDone." if code == 0 else f"\nFailed (exit {code}).")
|
|
||||||
t.quit()
|
|
||||||
def release():
|
|
||||||
# Runs (queued onto the GUI thread) only after QThread emitted
|
|
||||||
# finished. t.wait() then joins the last few instructions of the OS
|
|
||||||
# thread, so by the time the refs are dropped the thread is truly
|
|
||||||
# dead. Dropping them in finish() — right after t.quit() — crashed
|
|
||||||
# the app: the still-running QThread wrapper became garbage, and
|
|
||||||
# collecting a live QThread aborts the process ("QThread: Destroyed
|
|
||||||
# while thread is still running"). Reproduced on every
|
|
||||||
# compress-with-key run; this ordering is the fix.
|
|
||||||
t.wait()
|
|
||||||
parent._jobs = [(th, wk) for (th, wk) in parent._jobs if th is not t]
|
|
||||||
# `done` is emitted from the worker thread and `finish` touches GUI widgets;
|
|
||||||
# a bare functor would connect DirectConnection and run OFF the GUI thread.
|
|
||||||
# QueuedConnection marshals it onto the GUI event loop.
|
|
||||||
w.done.connect(finish, Qt.ConnectionType.QueuedConnection)
|
|
||||||
t.finished.connect(release, Qt.ConnectionType.QueuedConnection)
|
|
||||||
t.started.connect(w.run); t.start()
|
|
||||||
|
|
||||||
# ── Tabs ──
|
# ── Tabs ──
|
||||||
|
|
||||||
|
|
@ -936,8 +976,8 @@ class ZuptWindow(QMainWindow):
|
||||||
# child CLI process (the worker then sees EOF and finishes) and wait
|
# child CLI process (the worker then sees EOF and finishes) and wait
|
||||||
# for its QThread. Otherwise interpreter teardown collects live
|
# for its QThread. Otherwise interpreter teardown collects live
|
||||||
# QThreads and aborts the process instead of exiting cleanly.
|
# QThreads and aborts the process instead of exiting cleanly.
|
||||||
jobs = [(t, w) for i in range(self.tabs.count())
|
jobs = [j for i in range(self.tabs.count())
|
||||||
for (t, w) in getattr(self.tabs.widget(i), "_jobs", [])]
|
for j in list(getattr(self.tabs.widget(i), "_jobs", []))]
|
||||||
if jobs:
|
if jobs:
|
||||||
# Aborting mid-job can be destructive (a killed `disk restore`
|
# Aborting mid-job can be destructive (a killed `disk restore`
|
||||||
# leaves the target half-written), so never do it silently.
|
# leaves the target half-written), so never do it silently.
|
||||||
|
|
@ -947,10 +987,8 @@ class ZuptWindow(QMainWindow):
|
||||||
"An operation is still running.\nQuit and abort it?",
|
"An operation is still running.\nQuit and abort it?",
|
||||||
SB.Yes | SB.Cancel) != SB.Yes:
|
SB.Yes | SB.Cancel) != SB.Yes:
|
||||||
e.ignore(); return
|
e.ignore(); return
|
||||||
for t, w in jobs:
|
for j in jobs:
|
||||||
w.cancel()
|
if not j.cancel_and_join(3000):
|
||||||
t.quit()
|
|
||||||
if not t.wait(3000):
|
|
||||||
# Thread stuck past the kill (child in D-state / pipe held by a
|
# Thread stuck past the kill (child in D-state / pipe held by a
|
||||||
# grandchild). Letting teardown destroy a live QThread aborts
|
# grandchild). Letting teardown destroy a live QThread aborts
|
||||||
# with SIGABRT; exiting hard here is the clean way out.
|
# with SIGABRT; exiting hard here is the clean way out.
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue