fix: QThread crash on iPod connect and device detection — proper wait() lifecycle

Root cause: QThread C++ destructor ran while underlying OS thread was
still terminating. The pattern of checking isRunning() and using
short timeouts (wait(3000/5000)) left a window where the Python
reference was dropped and Shiboken destroyed the C++ object before
the thread fully exited.

Fixes:
- Remove all self.finished.connect(self.deleteLater) — unsafe,
  deleteLater from within run() or from worker thread races with
  thread termination
- _cleanup_worker: always call wait() (no timeout) before nulling
  the Python reference — guarantees OS thread is fully dead
- Add _is_worker_running() helper with try/except RuntimeError
  guard to safely check stale C++ objects
- DeviceMonitor._poll/stop/wait: use wait() with no timeout
- closeEvent: guard isRunning() with try/except RuntimeError
This commit is contained in:
Maksim Totmin 2026-06-01 19:25:30 +07:00
parent dd93440146
commit a36312c9d7
5 changed files with 63 additions and 24 deletions

View File

@ -66,7 +66,6 @@ class MainWindow(QMainWindow):
self.library_tab._scan_library() self.library_tab._scan_library()
self._resume_playback_if_saved() self._resume_playback_if_saved()
self._restore_library_state() self._restore_library_state()
self.ipod_tab._on_refresh_devices_clicked()
self._start_device_monitor() self._start_device_monitor()
def _setup_player(self): def _setup_player(self):
@ -427,7 +426,17 @@ class MainWindow(QMainWindow):
def closeEvent(self, event): def closeEvent(self, event):
if self.device_monitor: if self.device_monitor:
self.device_monitor.stop() self.device_monitor.stop()
self.ipod_tab.stop_monitoring(self.device_monitor)
for tab in (self.ipod_tab, self.library_tab):
if hasattr(tab, 'worker_thread') and tab.worker_thread:
try:
if tab.worker_thread.isRunning():
tab.worker_thread.stop()
except RuntimeError:
tab.worker_thread = None
self._save_settings() self._save_settings()
from PyQt6.QtWidgets import QApplication
QApplication.processEvents()
event.accept() event.accept()

View File

@ -11,6 +11,9 @@ logger = logging.getLogger(__name__)
class _DetectWorker(QThread): class _DetectWorker(QThread):
finished = pyqtSignal(object) finished = pyqtSignal(object)
def __init__(self, parent=None):
super().__init__(parent)
def run(self): def run(self):
try: try:
devices = IPodDevice().detect_devices() devices = IPodDevice().detect_devices()
@ -38,19 +41,29 @@ class DeviceMonitor(QObject):
def stop(self): def stop(self):
self.timer.stop() self.timer.stop()
if self._poll_worker and self._poll_worker.isRunning(): if self._poll_worker:
self._poll_worker.quit() self._poll_worker.quit()
self._poll_worker.wait(3000) self._poll_worker.wait()
self._poll_worker = None
self._device_ids.clear() self._device_ids.clear()
def _poll(self): def _poll(self):
if self._poll_worker and self._poll_worker.isRunning(): if self._poll_worker:
try:
if self._poll_worker.isRunning():
return return
self._poll_worker = _DetectWorker() except RuntimeError:
self._poll_worker.finished.connect(self._on_devices_detected) self._poll_worker = None
self._poll_worker.start() worker = _DetectWorker()
worker.finished.connect(self._on_devices_detected)
worker.start()
self._poll_worker = worker
def _on_devices_detected(self, devices): def _on_devices_detected(self, devices):
if self._poll_worker:
self._poll_worker.quit()
self._poll_worker.wait()
self._poll_worker = None
current_ids = {d["id"] for d in devices} current_ids = {d["id"] for d in devices}
if current_ids != self._device_ids: if current_ids != self._device_ids:
self._device_ids = current_ids self._device_ids = current_ids

View File

@ -107,7 +107,7 @@ class iPodTab(QWidget):
layout.addWidget(self.delete_status) layout.addWidget(self.delete_status)
def _on_refresh_devices_clicked(self): def _on_refresh_devices_clicked(self):
if self.worker_thread and self.worker_thread.isRunning(): if self._is_worker_running():
return return
self.device_combo.clear() self.device_combo.clear()
@ -122,6 +122,8 @@ class iPodTab(QWidget):
self.device_info_label.setText(status) self.device_info_label.setText(status)
def _on_device_detection_finished(self, success, message, result): def _on_device_detection_finished(self, success, message, result):
self._cleanup_worker()
if success: if success:
self.ipod_devices = result self.ipod_devices = result
self.device_combo.clear() self.device_combo.clear()
@ -140,8 +142,6 @@ class iPodTab(QWidget):
else: else:
self._set_device_not_found() self._set_device_not_found()
self._cleanup_worker()
def _start_mount_and_load(self, device: dict, mount_point=None, already_mounted=False): def _start_mount_and_load(self, device: dict, mount_point=None, already_mounted=False):
self.device_status_label.setText("Mounting and loading tracks...") self.device_status_label.setText("Mounting and loading tracks...")
self.mount_button.setEnabled(False) self.mount_button.setEnabled(False)
@ -320,11 +320,12 @@ class iPodTab(QWidget):
if reply != QMessageBox.StandardButton.Yes: if reply != QMessageBox.StandardButton.Yes:
return return
if self.worker_thread and self.worker_thread.isRunning(): if self._is_worker_running():
QMessageBox.information(self, "Info", "Please wait for the current task to finish") QMessageBox.information(self, "Info", "Please wait for the current task to finish")
return return
pids = [] pids = []
for item in selected: for item in selected:
track_data = item.data(Qt.ItemDataRole.UserRole) track_data = item.data(Qt.ItemDataRole.UserRole)
if track_data and "pid" in track_data: if track_data and "pid" in track_data:
@ -391,7 +392,7 @@ class iPodTab(QWidget):
QMessageBox.warning(self, "Error", "No valid track files found on device") QMessageBox.warning(self, "Error", "No valid track files found on device")
return return
if self.worker_thread and self.worker_thread.isRunning(): if self._is_worker_running():
QMessageBox.information(self, "Info", "Please wait for the current task to finish") QMessageBox.information(self, "Info", "Please wait for the current task to finish")
return return
@ -470,7 +471,7 @@ class iPodTab(QWidget):
QMessageBox.warning(self, "Error", "No iPod device mounted.") QMessageBox.warning(self, "Error", "No iPod device mounted.")
return return
if self.worker_thread and self.worker_thread.isRunning(): if self._is_worker_running():
QMessageBox.information(self, "Info", "Please wait for the current task to finish") QMessageBox.information(self, "Info", "Please wait for the current task to finish")
return return
@ -506,12 +507,20 @@ class iPodTab(QWidget):
pass pass
def _on_auto_state_changed(self, devices): def _on_auto_state_changed(self, devices):
if self.worker_thread and self.worker_thread.isRunning(): if self._is_worker_running():
return return
self._on_device_detection_finished(True, f"Found {len(devices)} devices", devices) self._on_device_detection_finished(True, f"Found {len(devices)} devices", devices)
def _cleanup_worker(self): def _cleanup_worker(self):
if self.worker_thread: if self.worker_thread:
self.worker_thread.wait(3000) self.worker_thread.quit()
self.worker_thread.deleteLater() self.worker_thread.wait()
self.worker_thread = None self.worker_thread = None
def _is_worker_running(self):
if self.worker_thread:
try:
return self.worker_thread.isRunning()
except RuntimeError:
self.worker_thread = None
return False

View File

@ -957,7 +957,7 @@ class LibraryTab(QWidget):
) )
return return
if self.worker_thread and self.worker_thread.isRunning(): if self._is_worker_running():
QMessageBox.information(self, "Info", "Please wait for the current task to finish") QMessageBox.information(self, "Info", "Please wait for the current task to finish")
return return
@ -1025,7 +1025,7 @@ class LibraryTab(QWidget):
QMessageBox.warning(self, "Error", "No iPod device mounted. Go to iPod tab and mount first.") QMessageBox.warning(self, "Error", "No iPod device mounted. Go to iPod tab and mount first.")
return return
if self.worker_thread and self.worker_thread.isRunning(): if self._is_worker_running():
QMessageBox.information(self, "Info", "Please wait for the current task to finish") QMessageBox.information(self, "Info", "Please wait for the current task to finish")
return return
@ -1070,7 +1070,7 @@ class LibraryTab(QWidget):
QMessageBox.warning(self, "Error", "No iPod device mounted.") QMessageBox.warning(self, "Error", "No iPod device mounted.")
return return
if self.worker_thread and self.worker_thread.isRunning(): if self._is_worker_running():
QMessageBox.information(self, "Info", "Please wait for the current task to finish") QMessageBox.information(self, "Info", "Please wait for the current task to finish")
return return
@ -1140,10 +1140,18 @@ class LibraryTab(QWidget):
def _cleanup_worker(self): def _cleanup_worker(self):
if self.worker_thread: if self.worker_thread:
self.worker_thread.wait(3000) self.worker_thread.quit()
self.worker_thread.deleteLater() self.worker_thread.wait()
self.worker_thread = None self.worker_thread = None
def _is_worker_running(self):
if self.worker_thread:
try:
return self.worker_thread.isRunning()
except RuntimeError:
self.worker_thread = None
return False
def on_device_mounted(self, mount_point: str): def on_device_mounted(self, mount_point: str):
self._current_mount_point = mount_point self._current_mount_point = mount_point
self.library_progress.setVisible(True) self.library_progress.setVisible(True)

View File

@ -27,8 +27,8 @@ class WorkerThread(QThread):
progress_signal = pyqtSignal(int, str) progress_signal = pyqtSignal(int, str)
finished_signal = pyqtSignal(bool, str, object) finished_signal = pyqtSignal(bool, str, object)
def __init__(self, task_type: str, **kwargs): def __init__(self, task_type: str, parent=None, **kwargs):
super().__init__() super().__init__(parent)
self.task_type = task_type self.task_type = task_type
self.kwargs = kwargs self.kwargs = kwargs
self.is_running = True self.is_running = True