fix: shut down worker QThreads safely on close

PyMemoryEditor/app/_widgets.py

@@ -6,12 +6,60 @@
 previously appeared duplicated across several dialog modules.
 """
 
-from typing import Optional
+from typing import List, Optional
 
-from PySide6.QtCore import Qt
+from PySide6.QtCore import Qt, QThread
 from PySide6.QtGui import QStandardItem
 
 
+# Workers that wouldn't stop in time on close are parked here so they are never
+# destroyed while still running (that aborts the whole process with
+# "QThread: Destroyed while thread is still running"). The list is module-level
+# so it outlives the dialog that owned the worker. Each detached worker removes
+# itself once it finishes.
+_DETACHED_WORKERS: "List[QThread]" = []
+
+
+def shutdown_worker_thread(worker: Optional[QThread], wait_ms: int = 2000) -> None:
+    """Stop a dialog's background ``QThread`` safely.
+
+    Disconnects every signal first (so a late emit can't touch a now-closing
+    dialog), asks the worker to cancel, then waits up to ``wait_ms``. If the
+    worker is still wedged in a backend call when the wait expires, it is
+    *detached* — reparented away from the dialog and held in a module-level
+    list — so the dialog can be destroyed without taking a live thread down
+    with it. The worker drops itself from the list once it finishes.
+
+    The worker is expected to expose a ``cancel()`` method (all of this app's
+    workers do); it's called if present.
+    """
+    if worker is None:
+        return
+    cancel = getattr(worker, "cancel", None)
+    if callable(cancel):
+        cancel()
+    try:
+        worker.disconnect()  # drop every outgoing connection at once
+    except (RuntimeError, TypeError):
+        pass
+
+    if worker.isRunning():
+        worker.wait(wait_ms)
+
+    if worker.isRunning():
+        worker.setParent(None)
+        _DETACHED_WORKERS.append(worker)
+        worker.finished.connect(lambda: _reap_detached_worker(worker))
+    else:
+        worker.deleteLater()
+
+
+def _reap_detached_worker(worker: QThread) -> None:
+    if worker in _DETACHED_WORKERS:
+        _DETACHED_WORKERS.remove(worker)
+    worker.deleteLater()
+
+
 class NumericItem(QStandardItem):
     """A QStandardItem that compares by its Qt.UserRole int payload.
 

PyMemoryEditor/app/cheat_table.py

@@ -280,6 +280,10 @@ def _on_cell_changed(self, row: int, column: int) -> None:
 
         entry = self._entries[row]
         item = self._table.item(row, column)
+        if item is None:
+            # cellChanged can fire mid-teardown for a cell whose item was
+            # already cleared; nothing to read.
+            return
 
         if column == self.COL_ACTIVE:
             entry.frozen = item.checkState() == Qt.Checked

PyMemoryEditor/app/main_window.py

@@ -22,7 +22,7 @@
 
 import psutil
 
-from PySide6.QtCore import Qt, QSettings, QTimer, Signal
+from PySide6.QtCore import Qt, QEventLoop, QSettings, QThread, QTimer, Signal
 from PySide6.QtGui import QAction, QActionGroup, QCloseEvent, QKeySequence
 from PySide6.QtWidgets import (
     QApplication,
@@ -81,6 +81,9 @@ def __init__(self, process: AbstractProcess):
         super().__init__()
         self._process = process
         self._worker: Optional[Union[FirstScanWorker, RefineScanWorker]] = None
+        # Workers that wouldn't stop in time on shutdown are detached here so
+        # the closing window can't destroy a still-running QThread.
+        self._zombie_workers: List[QThread] = []
         self._region_snapshot: Optional[list] = None
         self._memory_map: Optional[MemoryMapDialog] = None
         self._hex_viewers: List[MemoryViewerDialog] = []
@@ -781,10 +784,10 @@ def _change_process(self) -> None:
         if picker.exec() != picker.DialogCode.Accepted or picker.process is None:
             return
 
-        try:
-            self._process.close()
-        except Exception:
-            pass
+        # Keep the old handle open until the old cheat poller has been joined
+        # (see below). Closing it here would let the still-running poller thread
+        # read from a closed handle in the window before shutdown().
+        old_process = self._process
 
         self._process = picker.process
         self._proc_name = self._read_proc_name()
@@ -817,6 +820,11 @@ def _change_process(self) -> None:
             old_cheat.shutdown()
         except Exception:
             pass
+        # The old poller is joined now, so it's safe to release the old handle.
+        try:
+            old_process.close()
+        except Exception:
+            pass
         old_index = self._right_splitter.indexOf(old_cheat)
         self._cheat = CheatTable(self._process)
         self._cheat.pointer_scan_for_address.connect(self._open_pointer_scan_dialog)
@@ -830,6 +838,11 @@ def _change_process(self) -> None:
         self._status.showMessage(f"Now targeting PID {self._process.pid}.")
 
     def closeEvent(self, event: QCloseEvent) -> None:
+        # Stop the heartbeat *first*: _shutdown_worker pumps the event loop
+        # below, and a heartbeat firing during that pump could pop a modal
+        # dialog mid-teardown.
+        self._heartbeat.stop()
+
         # The cheat poller is a child widget but its closeEvent is *not*
         # guaranteed to fire on a top-level window close — only on an explicit
         # close of the child. Drive it directly so the QThread is stopped
@@ -840,22 +853,50 @@ def closeEvent(self, event: QCloseEvent) -> None:
             pass
 
         self._shutdown_worker()
-        self._heartbeat.stop()
+
+        # Close worker-bearing auxiliary dialogs so their own closeEvent stops
+        # and joins the background thread. Otherwise the imminent destruction of
+        # this window would destroy those still-running QThreads and abort the
+        # process ("QThread: Destroyed while thread is still running").
+        for attr in (
+            "_memory_map",
+            "_threads_dialog",
+            "_modules_dialog",
+            "_pointer_scan_dialog",
+        ):
+            dialog = getattr(self, attr, None)
+            if dialog is not None:
+                dialog.close()
+
         self.closing.emit()
         super().closeEvent(event)
 
     def _shutdown_worker(self) -> None:
         """
         Stop the active scan worker safely.
 
-        Disconnect every signal **before** waiting — if the wait times out
-        (long scan that ignores cancel), a late ``chunk_ready`` / ``finished``
-        emit would otherwise land on a half-destroyed UI. Then wait the
-        capped time and forcibly forget the worker either way.
+        Two hazards make a naive ``worker.wait()`` here both deadlock-prone and
+        crash-prone:
+
+        * ``chunk_ready`` is a *BlockingQueuedConnection* — the worker parks
+          inside ``emit()`` until this (GUI) thread services the slot. If we
+          just call ``wait()`` the GUI thread is stuck in ``wait()`` and can
+          never run the event loop to release the worker → deadlock until the
+          timeout. We therefore *pump the event loop* while waiting.
+        * Destroying a still-running ``QThread`` aborts the process
+          ("QThread: Destroyed while thread is still running"). If the worker
+          is wedged in a long backend syscall and won't stop in time, we must
+          **not** let the closing window take it down — we detach it and keep a
+          reference until it finishes on its own.
+
+        Disconnect every signal up front: a late ``chunk_ready``/``finished``
+        must not touch the closing UI, and ``finished`` must not kick off the
+        ``_fill_initial_values`` follow-up scan during teardown.
         """
         worker = self._worker
         if worker is None:
             return
+        self._worker = None
         worker.cancel()
         try:
             worker.chunk_ready.disconnect()
@@ -867,8 +908,32 @@ def _shutdown_worker(self) -> None:
         except (RuntimeError, TypeError):
             # Already disconnected / no slots — fine, we just want them gone.
             pass
-        worker.wait(_WORKER_SHUTDOWN_WAIT_MS)
-        self._worker = None
+
+        # Pump queued cross-thread calls so any in-flight blocking emit is
+        # dispatched (which releases the worker even though we just
+        # disconnected the slot) and the worker can observe cancel() and unwind.
+        waited = 0
+        step_ms = 25
+        while worker.isRunning() and waited < _WORKER_SHUTDOWN_WAIT_MS:
+            QApplication.processEvents(QEventLoop.ProcessEventsFlag.AllEvents, step_ms)
+            worker.wait(step_ms)
+            waited += step_ms
+
+        if worker.isRunning():
+            # Still wedged in a backend call. Detach it from this window so the
+            # imminent window destruction can't destroy a live QThread, and
+            # keep it referenced until it finishes on its own.
+            worker.setParent(None)
+            self._zombie_workers.append(worker)
+            worker.finished.connect(lambda: self._reap_worker(worker))
+        else:
+            worker.deleteLater()
+
+    def _reap_worker(self, worker: QThread) -> None:
+        """Drop our reference to a detached worker once it has finished."""
+        if worker in self._zombie_workers:
+            self._zombie_workers.remove(worker)
+        worker.deleteLater()
 
 
 def _safe_for_json(value) -> object:

PyMemoryEditor/app/memory_map_dialog.py

@@ -37,7 +37,7 @@
 
 from PyMemoryEditor import AbstractProcess, MemoryRegion, MemoryRegionSnapshot
 
-from ._widgets import NumericItem
+from ._widgets import NumericItem, shutdown_worker_thread
 
 
 class _SnapshotWorker(QThread):
@@ -471,7 +471,8 @@ def _populate(self) -> None:
     def _select_address(self, address: int, *, scroll: bool = True) -> None:
         """Select the row whose base address equals ``address`` (if present)."""
         for row in range(self._model.rowCount()):
-            if self._model.item(row, 0).data(Qt.UserRole) == address:
+            item = self._model.item(row, 0)
+            if item is not None and item.data(Qt.UserRole) == address:
                 self._table.selectRow(row)
                 if scroll:
                     self._table.scrollTo(self._model.index(row, 0))
@@ -491,25 +492,21 @@ def _on_worker_finished(self) -> None:
 
     def closeEvent(self, event):  # noqa: N802 — Qt naming
         self._auto_timer.stop()
-        # If the snapshot is still in flight, let it finish without holding
-        # the UI hostage but unhook our slots so a late emit doesn't touch
-        # a destroyed dialog.
-        if self._worker is not None and self._worker.isRunning():
-            try:
-                self._worker.snapshot_ready.disconnect()
-                self._worker.snapshot_failed.disconnect()
-                self._worker.finished.disconnect()
-            except (RuntimeError, TypeError):
-                pass
-            self._worker.wait(1000)
+        # Unhook + join the snapshot worker; if it can't stop in time it's
+        # detached rather than destroyed under us.
+        shutdown_worker_thread(self._worker, wait_ms=1000)
+        self._worker = None
         super().closeEvent(event)
 
     def _selected_region(self) -> Optional[MemoryRegion]:
         rows = self._table.selectionModel().selectedRows()
         if not rows:
             return None
         row = rows[0].row()
-        addr = int(self._model.item(row, 0).data(Qt.UserRole))
+        item = self._model.item(row, 0)
+        if item is None:
+            return None
+        addr = int(item.data(Qt.UserRole))
         # Look up the real MemoryRegion from the snapshot so callers get the
         # full set of fields (is_writable, struct, path, …). Falls back to a
         # minimal stub if the row's address is no longer in the snapshot

PyMemoryEditor/app/memory_viewer_dialog.py

@@ -155,6 +155,11 @@ def refresh(self) -> None:
         size = int(self._size_spin.value())
         try:
             data = self._process.read_process_memory(addr, bytes, size)
+            # The conversion/format must stay inside the guard: a backend that
+            # returns a non-buffer object would make bytes(data) raise TypeError,
+            # which (outside the try) escapes this slot and crashes the app.
+            if not isinstance(data, (bytes, bytearray)):
+                data = bytes(data)
         except Exception as exc:  # noqa: BLE001 — surface every backend error
             self._dump.setPlainText("")
             self._status.setText(f"Read failed: {type(exc).__name__}: {exc}")
@@ -167,8 +172,6 @@ def refresh(self) -> None:
             )
             return
 
-        if not isinstance(data, (bytes, bytearray)):
-            data = bytes(data)
         self._dump.setPlainText(_format_hex_dump(addr, bytes(data)))
         self._status.setText(f"Read {len(data):,} bytes from 0x{addr:X}")
 

PyMemoryEditor/app/modules_dialog.py

@@ -38,7 +38,7 @@
 
 from PyMemoryEditor import AbstractProcess, ModuleInfo
 
-from ._widgets import NumericItem
+from ._widgets import NumericItem, shutdown_worker_thread
 from .memory_map_dialog import _format_size
 
 
@@ -239,7 +239,8 @@ def _apply_filter(self) -> None:
     def _select_address(self, address: int) -> None:
         """Re-select the row whose base address matches (no scrolling)."""
         for row in range(self._model.rowCount()):
-            if self._model.item(row, 1).data(Qt.UserRole) == address:
+            item = self._model.item(row, 1)
+            if item is not None and item.data(Qt.UserRole) == address:
                 self._table.selectRow(row)
                 return
 
@@ -260,8 +261,12 @@ def _selected_module(self) -> Optional[dict]:
         if not rows:
             return None
         row = rows[0].row()
-        base = self._model.item(row, 1).data(Qt.UserRole)
-        size = self._model.item(row, 2).data(Qt.UserRole)
+        base_item = self._model.item(row, 1)
+        size_item = self._model.item(row, 2)
+        if base_item is None or size_item is None:
+            return None
+        base = base_item.data(Qt.UserRole)
+        size = size_item.data(Qt.UserRole)
         return {"base_address": int(base), "size": int(size)}
 
     def _show_context_menu(self, pos) -> None:
@@ -323,14 +328,8 @@ def _emit_hex_viewer_request(self) -> None:
 
     def closeEvent(self, event):  # noqa: N802 — Qt naming
         self._auto_timer.stop()
-        # If the enumeration is still in flight, let it finish but unhook our
-        # slots so a late emit doesn't touch a destroyed dialog.
-        if self._worker is not None and self._worker.isRunning():
-            try:
-                self._worker.modules_ready.disconnect()
-                self._worker.modules_failed.disconnect()
-                self._worker.finished.disconnect()
-            except (RuntimeError, TypeError):
-                pass
-            self._worker.wait(1000)
+        # Unhook + join the enumeration worker; if it can't stop in time it's
+        # detached rather than destroyed under us.
+        shutdown_worker_thread(self._worker, wait_ms=1000)
+        self._worker = None
         super().closeEvent(event)

PyMemoryEditor/app/pointer_chain_dialog.py

@@ -501,6 +501,18 @@ def _on_resolve(self) -> None:
                 else f"base + sum of {len(offsets)} offset(s)"
             )
 
+        # The resolved address is later emitted through a "qulonglong" signal
+        # and formatted as hex; a negative or >= 2**64 value (raw mode with
+        # large/negative offsets) would raise OverflowError/ValueError. Reject
+        # it cleanly instead of crashing.
+        if not 0 <= resolved < 2 ** 64:
+            self._resolved_address = None
+            self._output_label.setText("Resolved address: <out of 64-bit range>")
+            self._value_label.setText("Value: —")
+            self._copy_addr_btn.setEnabled(False)
+            self._add_to_cheat_btn.setEnabled(False)
+            return
+
         self._resolved_address = resolved
         self._output_label.setText(
             f"Resolved address: 0x{resolved:X}  ({hop_summary})"

PyMemoryEditor/app/pointer_scan_dialog.py

@@ -52,7 +52,7 @@
 from PyMemoryEditor import AbstractProcess, PointerPath
 from PyMemoryEditor.process.pointer_scan import intersect_pointer_paths
 
-from ._widgets import NumericItem, parse_hex_address
+from ._widgets import NumericItem, parse_hex_address, shutdown_worker_thread
 from .value_types import VALUE_TYPES, ValueTypeSpec, find_spec
 
 
@@ -993,14 +993,9 @@ def _on_compare_saved(self) -> None:
         self._export_btn.setEnabled(True)
 
     def closeEvent(self, event):  # noqa: N802 — Qt naming
-        if self._worker is not None and self._worker.isRunning():
-            self._worker.cancel()
-            try:
-                self._worker.rows_ready.disconnect()
-                self._worker.progress.disconnect()
-                self._worker.status.disconnect()
-                self._worker.finished_ok.disconnect()
-            except (RuntimeError, TypeError):
-                pass
-            self._worker.wait(2000)
+        # Unhook every signal (the previous code missed `error`/`finished`, so
+        # a late emit could land on a destroyed dialog) and join the worker; if
+        # it can't stop in time it's detached rather than destroyed under us.
+        shutdown_worker_thread(self._worker, wait_ms=2000)
+        self._worker = None
         super().closeEvent(event)