diff --git a/PyMemoryEditor/app/_auto_refresh_dialog.py b/PyMemoryEditor/app/_auto_refresh_dialog.py index d49ea76..f9aae5e 100644 --- a/PyMemoryEditor/app/_auto_refresh_dialog.py +++ b/PyMemoryEditor/app/_auto_refresh_dialog.py @@ -10,13 +10,24 @@ that self-throttles the poll, the detach-on-shutdown safety) where a fix to one copy wouldn't reach the others. This owns the lifecycle once; subclasses provide only what differs: how to fetch the data, and how to render it. + +It also owns *when* a failure is worth interrupting the user — see +:meth:`AutoRefreshTableDialog._handle_failed`. """ from typing import Callable, Optional -from PySide6.QtCore import QThread, QTimer, Signal +from PySide6.QtCore import QElapsedTimer, QThread, QTimer, Signal from PySide6.QtWidgets import QDialog -from ._widgets import shutdown_worker_thread +from ._widgets import TearsDownOnClose, shutdown_worker_thread + + +# How long a target may keep failing before the auto-refresh gives up. Not +# every failure is terminal (``CreateToolhelp32Snapshot`` is documented to fail +# transiently while a target loads images), so a blip must not freeze a window +# that would recover next tick. In wall-clock, not ticks: the intervals differ +# by more than 10x across the windows that share this budget. +_FAILURE_GRACE_MS = 3000 class _DataWorker(QThread): @@ -38,7 +49,7 @@ def run(self) -> None: self.ready.emit(data) -class AutoRefreshTableDialog(QDialog): +class AutoRefreshTableDialog(TearsDownOnClose, QDialog): """Owns the fetch-worker lifecycle + auto-refresh timer for a table dialog. Subclasses must: @@ -57,6 +68,13 @@ def __init__(self, process, *, refresh_interval_ms: int, parent=None): self._has_data = False self._refresh_interval_ms = refresh_interval_ms self._auto_timer: Optional[QTimer] = None + # Failure state, all reset by a success or an explicit refresh(). See + # _handle_failed for how they decide what reaches the user. + self._last_error: Optional[str] = None # doubles as "already reported" + self._failing_since: Optional[QElapsedTimer] = None # streak clock + # Read by subclasses that rebuild their status line on user input, so + # the "stopped" note isn't replaced by a count that looks live. + self._polling_gave_up = False def _start_auto_refresh(self) -> None: """Begin polling. Call once, after the first ``refresh()``. @@ -67,7 +85,7 @@ def _start_auto_refresh(self) -> None: """ self._auto_timer = QTimer(self) self._auto_timer.setInterval(self._refresh_interval_ms) - self._auto_timer.timeout.connect(self.refresh) + self._auto_timer.timeout.connect(self._auto_refresh_tick) self._auto_timer.start() # ------------------------------------------------------------------ # @@ -83,17 +101,55 @@ def _on_data_ready(self, data) -> None: raise NotImplementedError def _on_data_failed(self, message: str) -> None: - """Report a failed fetch. Runs on the UI thread.""" + """Report a failed fetch. Runs on the UI thread. + + Called only for a failure the user needs to see — the first fetch, or + the one that makes the poll give up — and never twice for the same + error (see :meth:`_handle_failed`), so a modal is safe here. + """ raise NotImplementedError def _set_loading_hint(self) -> None: """Optionally show a one-time loading message before the first fetch.""" + def _on_polling_stopped(self) -> None: + """Note that the auto-refresh gave up. Runs on the UI thread. + + Fires once, when a target has been failing for ``_FAILURE_GRACE_MS``. + The table is frozen on its last good data from here on, so subclasses + should say so. Optional. + """ + # ------------------------------------------------------------------ # # Lifecycle (shared) # ------------------------------------------------------------------ # def refresh(self) -> None: + """Fetch now, on the user's behalf. + + Clears the error latch and restarts a poll that repeated failures + stopped. The timer goes through :meth:`_auto_refresh_tick` instead, + which must *not* re-arm — and neither may a dismissed dialog, whose + teardown latch is one-way. + """ + if self._is_dismissed(): + return + self._last_error = None + self._failing_since = None + self._polling_gave_up = False + if self._auto_timer is not None and not self._auto_timer.isActive(): + self._auto_timer.start() + self._start_fetch() + + def _auto_refresh_tick(self) -> None: + """Timer-driven fetch — never re-arms after a failure.""" + self._start_fetch() + + def _start_fetch(self) -> None: + # A worker started after the teardown could never be joined again. + if self._is_dismissed(): + return + # Skip if a fetch is already in flight — the timer would otherwise stack # workers on a slow target. This self-throttles to the real fetch time. if self._worker is not None and self._worker.isRunning(): @@ -106,26 +162,83 @@ def refresh(self) -> None: worker = _DataWorker(self._fetch_data, self) worker.ready.connect(self._handle_ready) - worker.failed.connect(self._on_data_failed) - worker.finished.connect(self._on_worker_finished) + worker.failed.connect(self._handle_failed) + worker.finished.connect(lambda w=worker: self._on_worker_finished(w)) self._worker = worker worker.start() def _handle_ready(self, data) -> None: + if self._is_dismissed(): + return self._has_data = True + self._last_error = None + self._failing_since = None self._on_data_ready(data) - def _on_worker_finished(self) -> None: - worker = self._worker - self._worker = None - if worker is not None: - worker.deleteLater() + def _handle_failed(self, message: str) -> None: + """Handle a failed fetch: report it only when the user needs to know. + + Once the target is gone (or its handle was closed by File → Change + Process) *every* tick fails, and ``_on_data_failed`` pops a **modal** + dialog. A modal runs a nested event loop, so the timer fires again while + it is up and the failures stack — hundreds of focus-stealing dialogs the + user could only escape by killing the app from another TTY (issue #74). + + Reporting is therefore tied to the two moments where a failure changes + what the user sees: the **first** fetch (empty window) and the one that + makes the poll **give up** (frozen table). Everything in between heals + quietly — and that silence is load-bearing: reporting every failure + re-opens the spam, since a target failing on alternating ticks stacks a + modal per failure until the stack runs out. + + A dismissed dialog reports nothing. The teardown disconnects the worker, + but a delivery already queued then must not raise a modal over a window + that is gone — during ``MainWindow.closeEvent`` that would be a nested + event loop in the middle of teardown. + """ + if self._is_dismissed(): + return + + if self._failing_since is None: + self._failing_since = QElapsedTimer() + self._failing_since.start() + gave_up = False + else: + gave_up = ( + self._failing_since.elapsed() >= _FAILURE_GRACE_MS + and self._auto_timer is not None + and self._auto_timer.isActive() + ) + + if gave_up and self._auto_timer is not None: + self._auto_timer.stop() + self._polling_gave_up = True + + never_reported_yet = not self._has_data and self._last_error is None + if (gave_up or never_reported_yet) and message != self._last_error: + self._last_error = message + self._on_data_failed(message) + + # After the report, so the "stopped" note is what stays on screen. + if gave_up: + self._on_polling_stopped() + + def _on_worker_finished(self, worker) -> None: + """Retire the worker that just finished — and only that one. + + ``finished`` is queued, so it can arrive after the next tick already + started a replacement. Clearing ``self._worker`` blindly would drop the + reference to that *running* worker and ``deleteLater()`` it — destroying + a live QThread aborts the process. + """ + if worker is self._worker: + self._worker = None + worker.deleteLater() - def closeEvent(self, event): # noqa: N802 — Qt naming + def _teardown(self) -> None: if self._auto_timer is not None: self._auto_timer.stop() # Unhook + 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=1000) self._worker = None - super().closeEvent(event) diff --git a/PyMemoryEditor/app/_widgets.py b/PyMemoryEditor/app/_widgets.py index d0f5d1a..8234fd0 100644 --- a/PyMemoryEditor/app/_widgets.py +++ b/PyMemoryEditor/app/_widgets.py @@ -8,7 +8,7 @@ from typing import Any, Callable, Iterable, List, Optional, Tuple -from PySide6.QtCore import Qt, QThread +from PySide6.QtCore import QObject, Qt, QThread from PySide6.QtGui import QStandardItem @@ -44,7 +44,11 @@ def shutdown_worker_thread(worker: Optional[QThread], wait_ms: int = 2000) -> No if callable(cancel): cancel() try: - worker.disconnect() # drop every outgoing connection at once + # The four-argument static form on purpose: bare `worker.disconnect()` + # raises TypeError in PySide6 (so this except swallowed it and nothing + # was disconnected), and `worker.disconnect(worker)` only drops + # connections whose *receiver* is the worker — ours belong to the dialog. + QObject.disconnect(worker, None, None, None) except (RuntimeError, TypeError): pass @@ -52,19 +56,73 @@ def shutdown_worker_thread(worker: Optional[QThread], wait_ms: int = 2000) -> No worker.wait(wait_ms) if worker.isRunning(): - worker.setParent(None) - _DETACHED_WORKERS.append(worker) - worker.finished.connect(lambda: _reap_detached_worker(worker)) + detach_worker(worker) else: worker.deleteLater() +def detach_worker(worker: QThread) -> None: + """Park a worker that wouldn't stop, so its owner can be destroyed. + + Reparents it away from the dying widget and holds it in the module-level + registry until it finishes on its own. Callers that stop workers their own + way park them here too, so threads that outlived their owner are all in one + place. + """ + worker.setParent(None) + _DETACHED_WORKERS.append(worker) + worker.finished.connect(lambda: _reap_detached_worker(worker)) + # A worker that finished before this connect already emitted, so the reaper + # would never run and it would sit here for good. + if worker.isFinished(): + _reap_detached_worker(worker) + + def _reap_detached_worker(worker: QThread) -> None: if worker in _DETACHED_WORKERS: _DETACHED_WORKERS.remove(worker) worker.deleteLater() +class TearsDownOnClose: + """Mixin that runs a dialog's teardown on *every* way out, not just close. + + ``QDialog`` delivers a ``QCloseEvent`` only for a real close. The app's own + "Close" buttons call ``accept()`` and Esc reaches ``reject()``; both land in + ``done()``, which hides the dialog and emits ``finished`` *without* any + close event — so a teardown living in ``closeEvent`` never ran, and the + dialog kept polling, hidden and unreachable. + + Subclasses implement :meth:`_teardown`; the mixin runs it exactly once, + whichever way the dialog is dismissed, and must precede ``QDialog`` in the + bases so its overrides win. Running once makes instances single-use, which + is what every caller here assumes. + """ + + def _teardown(self) -> None: + """Release timers and background threads. Runs exactly once.""" + raise NotImplementedError + + def _is_dismissed(self) -> bool: + """Whether the teardown has run. Guards live on this, not on a repeated + attribute lookup a typo could silently disable.""" + return getattr(self, "_teardown_done", False) + + def _run_teardown_once(self) -> None: + if self._is_dismissed(): + return + self._teardown_done = True + self._teardown() + + def done(self, result: int) -> None: # noqa: N802 — Qt naming + self._run_teardown_once() + super().done(result) + + def closeEvent(self, event) -> None: # noqa: N802 — Qt naming + self._run_teardown_once() + super().closeEvent(event) + + class NumericItem(QStandardItem): """A QStandardItem that compares by its Qt.UserRole int payload. diff --git a/PyMemoryEditor/app/application.py b/PyMemoryEditor/app/application.py index 8841d4b..bcc65ba 100644 --- a/PyMemoryEditor/app/application.py +++ b/PyMemoryEditor/app/application.py @@ -539,7 +539,9 @@ def main(argv=None): app.exec() finally: try: - process.close() + # `process` is only the handle the picker opened; File → Change + # Process… released that one and attached the window to another. + window.process.close() except Exception: pass diff --git a/PyMemoryEditor/app/cheat_poll_worker.py b/PyMemoryEditor/app/cheat_poll_worker.py index 0abcda1..790f916 100644 --- a/PyMemoryEditor/app/cheat_poll_worker.py +++ b/PyMemoryEditor/app/cheat_poll_worker.py @@ -106,6 +106,11 @@ def request_write( with QMutexLocker(self._mutex): self._pending_writes.append((address, pytype, length, value)) + def _stopping(self) -> bool: + """Whether ``stop`` was asked for. Cheap enough to check per entry.""" + with QMutexLocker(self._mutex): + return self._stop + def stop(self) -> None: with QMutexLocker(self._mutex): self._stop = True @@ -126,6 +131,8 @@ def _drain_pending_writes(self) -> List[Tuple[int, type, int, str]]: failures: List[Tuple[int, type, int, str]] = [] for address, pytype, length, value in pending: + if self._stopping(): + break # the handle is about to go — see _poll_once try: self._process.write_process_memory(address, pytype, length, value) except Exception as exc: # noqa: BLE001 @@ -206,6 +213,11 @@ def _poll_once( values_by_address = None for address in addresses: + # Per entry, not per tick: shutdown() closes the handle right + # after stop(), and a write on a recycled one hits another + # process. + if self._stopping(): + return results frozen_value, is_frozen = freeze_by_addr[(pytype, length, address)] if values_by_address is not None: current = values_by_address.get(address) diff --git a/PyMemoryEditor/app/cheat_table.py b/PyMemoryEditor/app/cheat_table.py index 88878ef..c2d16f9 100644 --- a/PyMemoryEditor/app/cheat_table.py +++ b/PyMemoryEditor/app/cheat_table.py @@ -45,7 +45,7 @@ from PyMemoryEditor import AbstractProcess -from ._widgets import parse_hex_address +from ._widgets import parse_hex_address, shutdown_worker_thread from .cheat_entry import CheatEntry from .cheat_poll_worker import TICK_INTERVAL_MS, _CheatPollWorker from .value_types import VALUE_TYPES, ValueTypeSpec, find_spec, parse_value @@ -73,6 +73,8 @@ def __init__(self, process: AbstractProcess, parent=None): self._process = process self._entries: List[CheatEntry] = [] self._suspend_signals = False + # Before _build_ui(), which connects cellChanged — a path that reads it. + self._poller_stopped = False self._build_ui() @@ -101,12 +103,23 @@ def shutdown(self) -> None: ``closeEvent``, so ``_change_process`` and the top-level window's ``closeEvent`` need to drive the teardown explicitly to avoid leaving the poller running against a now-closed process handle. + + The join is bounded and a tick only checks the stop flag between + iterations, so a big table or a hung target can outlast it. That case + goes through ``shutdown_worker_thread``, which *detaches* the poller + instead of letting it be deleted with this widget — destroying a running + QThread aborts the process. """ # Stop the publish timer first so no fresh snapshot races our shutdown. if self._publish_timer.isActive(): self._publish_timer.stop() + # Idempotent: shutdown() is driven from several places, and handing the + # same worker over twice would touch an already-deleted object. + if self._poller_stopped: + return + self._poller_stopped = True self._poller.stop() - self._poller.wait(1000) + shutdown_worker_thread(self._poller, wait_ms=1000) def closeEvent(self, event): # noqa: N802 — Qt naming self.shutdown() @@ -317,6 +330,9 @@ def _on_cell_changed(self, row: int, column: int) -> None: # next poll tick reads the value back and corrects the cell if the # write didn't take, and an outright failure returns via # write_failed → _on_write_failed. + # Nothing to route it to once shutdown() handed the worker over. + if self._poller_stopped: + return self._poller.request_write( entry.address, entry.spec.pytype, entry.length, value ) @@ -327,6 +343,10 @@ def _on_cell_changed(self, row: int, column: int) -> None: def _publish_snapshot_to_worker(self) -> None: """Hand the worker a fresh immutable snapshot of every entry.""" + # A tick queued before shutdown() stopped the timer must not reach a + # worker being torn down. + if self._poller_stopped: + return snapshot = [ ( entry.address, diff --git a/PyMemoryEditor/app/log_console_dialog.py b/PyMemoryEditor/app/log_console_dialog.py index 380adc2..d5599b9 100644 --- a/PyMemoryEditor/app/log_console_dialog.py +++ b/PyMemoryEditor/app/log_console_dialog.py @@ -31,6 +31,8 @@ QVBoxLayout, ) +from ._widgets import TearsDownOnClose + _LOGGER_NAME = "PyMemoryEditor" @@ -71,7 +73,7 @@ def emit(self, record: logging.LogRecord) -> None: self.bridge.line_emitted.emit(line) -class LogConsoleDialog(QDialog): +class LogConsoleDialog(TearsDownOnClose, QDialog): """Live view of the PyMemoryEditor logger.""" # The dialog can be opened/closed repeatedly; we attach the handler on @@ -183,7 +185,7 @@ def _console_append(self, line: str) -> None: def _on_clear(self) -> None: self._console.clear() - def closeEvent(self, event): # noqa: N802 — Qt naming + def _teardown(self) -> None: # Detach the handler so the library doesn't keep emitting into a # dialog the user has dismissed. Don't lower the logger level past # what the caller had configured before us. @@ -195,4 +197,3 @@ def closeEvent(self, event): # noqa: N802 — Qt naming self._logger.removeHandler(self._handler) self._handler = None self._logger.setLevel(self._previous_level) - super().closeEvent(event) diff --git a/PyMemoryEditor/app/main_window.py b/PyMemoryEditor/app/main_window.py index bed4abe..cee207b 100644 --- a/PyMemoryEditor/app/main_window.py +++ b/PyMemoryEditor/app/main_window.py @@ -22,7 +22,7 @@ import psutil -from PySide6.QtCore import Qt, QEventLoop, QSettings, QThread, QTimer, Signal +from PySide6.QtCore import Qt, QEventLoop, QSettings, QTimer, Signal from PySide6.QtGui import QAction, QActionGroup, QCloseEvent, QKeySequence from PySide6.QtWidgets import ( QApplication, @@ -46,6 +46,7 @@ from PyMemoryEditor import AbstractProcess, __version__ from ._icon import app_icon +from ._widgets import detach_worker from .application import DEFAULT_THEME_ID, THEMES, apply_theme from .cheat_entry import CheatEntry from .cheat_table import CheatTable @@ -81,9 +82,6 @@ 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] = [] @@ -761,6 +759,12 @@ def _show_about(self) -> None: def _process_badge_text(self) -> str: return f"PID {self._process.pid} · {self._proc_name}" + @property + def process(self) -> AbstractProcess: + """The handle this window is currently attached to — not necessarily + the one it was constructed with, since File → Change Process… swaps it.""" + return self._process + def _window_title(self) -> str: # Qt prepends QGuiApplication.applicationDisplayName to the window # title on Windows/Linux; including it here would duplicate it. @@ -836,7 +840,6 @@ def _change_process(self) -> None: self._proc_create_time = self._read_proc_create_time() self.setWindowTitle(self._window_title()) self._process_badge.setText(self._process_badge_text()) - self._region_snapshot = None self._results_model.clear() self._scanner.set_has_results(False) # If the previous target exited, _check_process_alive locked the scanner @@ -845,8 +848,11 @@ def _change_process(self) -> None: self._scanner.set_busy(False) # Tear down auxiliary dialogs that hold a reference to the old - # process — reopening them rebuilds against the new target. + # process — reopening them rebuilds against the new target. The Memory + # Map belongs here too: it polls on a timer, so leaving it open left it + # reading the handle closed just below (issue #74). for dialog_attr in ( + "_memory_map", "_threads_dialog", "_modules_dialog", "_pointer_chain_dialog", @@ -856,6 +862,17 @@ def _change_process(self) -> None: if existing is not None: existing.close() setattr(self, dialog_attr, None) + + # Hex viewers poll the old handle too. They drop themselves from this + # list on `destroyed`, which only fires after this method returns — so + # the clear() is what actually empties it, not redundant. + for viewer in list(self._hex_viewers): + viewer.close() + self._hex_viewers.clear() + + # Cleared *after* the teardown: closing the Memory Map makes its + # `finished` handler adopt that dialog's snapshot — the old target's. + self._region_snapshot = None # Replace the cheat table — old entries point at the previous process. # QSplitter has no QLayout, so we use its native replaceWidget(index). old_cheat = self._cheat @@ -867,7 +884,9 @@ 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. + # Accepted window: a worker that blew its join is detached, not + # stopped, so it can still be inside a syscall here. The cheat poller + # writes, which is why it checks the stop flag per entry. try: old_process.close() except Exception: @@ -890,6 +909,26 @@ def closeEvent(self, event: QCloseEvent) -> None: # dialog mid-teardown. self._heartbeat.stop() + # Before _shutdown_worker for the same reason as the heartbeat: it + # pumps the loop, and a dialog still polling during that pump can pop a + # modal mid-teardown. Closing them also joins their threads. + for attr in ( + "_memory_map", + "_threads_dialog", + "_modules_dialog", + "_pointer_scan_dialog", + ): + dialog = getattr(self, attr, None) + if dialog is not None: + dialog.close() + + # Hex viewers run the same read worker and are children of this window, + # so destroying it would take a running QThread down with it. No clear() + # here: the list dies with the window. (Pointer Chain is absent from + # both lists on purpose — it resolves synchronously, no thread.) + for viewer in list(self._hex_viewers): + viewer.close() + # 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 @@ -901,20 +940,6 @@ def closeEvent(self, event: QCloseEvent) -> None: self._shutdown_worker() - # 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) @@ -967,21 +992,12 @@ def _shutdown_worker(self) -> None: 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)) + # Still wedged in a backend call: park it in the shared registry so + # the imminent window destruction can't take a live QThread down. + detach_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: if value is None or isinstance(value, (str, int, float, bool)): diff --git a/PyMemoryEditor/app/memory_map_dialog.py b/PyMemoryEditor/app/memory_map_dialog.py index ee28435..59f304e 100644 --- a/PyMemoryEditor/app/memory_map_dialog.py +++ b/PyMemoryEditor/app/memory_map_dialog.py @@ -414,15 +414,19 @@ def _populate(self) -> None: self._table.setSortingEnabled(True) if needle: - self._count_label.setText( + count_text = ( f"{shown:,} of {total:,} regions shown · " f"{_format_size(total_bytes)} mapped" ) else: - self._count_label.setText( + count_text = ( f"{total:,} regions · " f"{_format_size(total_bytes)} of virtual address space mapped" ) + # A filter keystroke rebuilds this line long after the poll gave up. + if self._polling_gave_up: + count_text += " · auto-refresh stopped" + self._count_label.setText(count_text) # Restore the view. A just-allocated region wins and is scrolled into # view; otherwise re-select whatever was selected before and keep the @@ -451,6 +455,12 @@ def _on_data_failed(self, message: str) -> None: self, "Memory Map", f"Failed to read memory regions:\n\n{message}" ) + def _on_polling_stopped(self) -> None: + self._count_label.setText( + "Failed to read memory regions — auto-refresh stopped. " + "Close and reopen this window to retry." + ) + def _selected_region(self) -> Optional[MemoryRegion]: rows = self._table.selectionModel().selectedRows() if not rows: diff --git a/PyMemoryEditor/app/memory_viewer_dialog.py b/PyMemoryEditor/app/memory_viewer_dialog.py index dfe37f3..3e61d70 100644 --- a/PyMemoryEditor/app/memory_viewer_dialog.py +++ b/PyMemoryEditor/app/memory_viewer_dialog.py @@ -8,7 +8,7 @@ import logging from typing import Optional -from PySide6.QtCore import QTimer +from PySide6.QtCore import QElapsedTimer, QTimer from PySide6.QtGui import QFont from PySide6.QtWidgets import ( QDialog, @@ -24,8 +24,8 @@ from PyMemoryEditor import AbstractProcess -from ._auto_refresh_dialog import _DataWorker -from ._widgets import parse_hex_address, shutdown_worker_thread +from ._auto_refresh_dialog import _FAILURE_GRACE_MS, _DataWorker +from ._widgets import TearsDownOnClose, parse_hex_address, shutdown_worker_thread # Child of the "PyMemoryEditor" logger, so the Log Console (which attaches a @@ -47,7 +47,7 @@ def _format_hex_dump(base: int, data: bytes) -> str: return "\n".join(lines) -class MemoryViewerDialog(QDialog): +class MemoryViewerDialog(TearsDownOnClose, QDialog): """Hex viewer + auto-refresh, with a "write bytes back" button.""" def __init__( @@ -146,6 +146,13 @@ def _build_ui(self) -> None: self._timer = QTimer(self) self._timer.timeout.connect(self.refresh) + # Failure bookkeeping, same shape as the auto-refresh dialogs: a dead + # target used to log a warning every tick forever (~20/s at the 50 ms + # floor). Keyed by (address, size, message) so moving the viewer to + # another failing range still gets logged. + self._last_failure: Optional[tuple] = None + self._failing_since: Optional[QElapsedTimer] = None + def _parse_address(self) -> Optional[int]: text = self._addr_edit.text().strip() if not text: @@ -195,37 +202,71 @@ def fetch(): worker = _DataWorker(fetch, self) worker.ready.connect(self._on_read_result) - worker.finished.connect(self._on_worker_finished) + worker.finished.connect(lambda w=worker: self._on_worker_finished(w)) self._worker = worker worker.start() def _on_read_result(self, result) -> None: """Render a finished read (UI thread). ``result`` is the fetch tuple.""" + # A metacall queued before _teardown disconnected still gets delivered. + if self._is_dismissed(): + return addr, size, data, exc = result if exc is not None: self._dump.setPlainText("") - self._status.setText(f"Read failed: {type(exc).__name__}: {exc}") - _LOG.warning( - "Hex viewer read failed at 0x%X (%d bytes): %s: %s", - addr, - size, - type(exc).__name__, - exc, + message = f"{type(exc).__name__}: {exc}" + + # First tick of a streak only. + if (addr, size, message) != self._last_failure: + self._last_failure = (addr, size, message) + _LOG.warning( + "Hex viewer read failed at 0x%X (%d bytes): %s", + addr, + size, + message, + ) + + gave_up = False + if self._failing_since is None: + self._failing_since = QElapsedTimer() + self._failing_since.start() + elif ( + self._failing_since.elapsed() >= _FAILURE_GRACE_MS + and self._auto_btn.isChecked() + ): + # Off through the button's own slot, so the UI can't claim to + # be refreshing while it isn't. + gave_up = True + self._auto_btn.setChecked(False) + + self._status.setText( + f"Read failed: {message}" + + (" — auto-refresh stopped." if gave_up else "") ) return + self._last_failure = None + self._failing_since = None self._dump.setPlainText(_format_hex_dump(addr, data)) self._status.setText(f"Read {len(data):,} bytes from 0x{addr:X}") - def _on_worker_finished(self) -> None: - worker = self._worker - self._worker = None - if worker is not None: - worker.deleteLater() + def _on_worker_finished(self, worker) -> None: + """Retire the worker that just finished — and only that one. + + ``finished`` is queued and can land after the next tick started a + replacement (50 ms interval floor here, so they overlap easily). + Clearing ``self._worker`` blindly would ``deleteLater()`` that running + worker — destroying a live QThread aborts the process. + """ + if worker is self._worker: + self._worker = None + worker.deleteLater() def _toggle_auto(self, on: bool) -> None: self._auto_btn.setText("Auto-refresh: On" if on else "Auto-refresh: Off") if on: + # Turning it back on is a retry: fresh grace window. + self._failing_since = None self._sync_timer() else: self._timer.stop() @@ -276,11 +317,10 @@ def _write_bytes(self) -> None: self._status.setText(f"Wrote {len(data)} bytes to 0x{addr:X}.") self.refresh() - def closeEvent(self, event) -> None: + def _teardown(self) -> None: self._timer.stop() # Unhook + join the read worker; if it's still wedged in a backend call # it's detached rather than destroyed under us (same safety the # auto-refresh dialogs use). shutdown_worker_thread(self._worker, wait_ms=1000) self._worker = None - super().closeEvent(event) diff --git a/PyMemoryEditor/app/modules_dialog.py b/PyMemoryEditor/app/modules_dialog.py index 3254778..aacf8a3 100644 --- a/PyMemoryEditor/app/modules_dialog.py +++ b/PyMemoryEditor/app/modules_dialog.py @@ -190,9 +190,13 @@ def _apply_filter(self) -> None: total = len(self._modules) if needle: - self._count_label.setText(f"{shown:,} of {total:,} module(s) shown") + count_text = f"{shown:,} of {total:,} module(s) shown" else: - self._count_label.setText(f"{total:,} module(s)") + count_text = f"{total:,} module(s)" + # A filter keystroke rebuilds this line long after the poll gave up. + if self._polling_gave_up: + count_text += " · auto-refresh stopped" + self._count_label.setText(count_text) # Restore the user's selection + scroll, so the periodic refresh doesn't # clear what they had highlighted or jump the table around. @@ -214,6 +218,12 @@ def _on_data_failed(self, message: str) -> None: self, "Modules", f"Failed to enumerate modules:\n\n{message}" ) + def _on_polling_stopped(self) -> None: + self._count_label.setText( + "Failed to enumerate modules — auto-refresh stopped. " + "Close and reopen this window to retry." + ) + def _selected_module(self) -> Optional[dict]: rows = self._table.selectionModel().selectedRows() if not rows: diff --git a/PyMemoryEditor/app/open_process_dialog.py b/PyMemoryEditor/app/open_process_dialog.py index e58fae4..dcdc2c5 100644 --- a/PyMemoryEditor/app/open_process_dialog.py +++ b/PyMemoryEditor/app/open_process_dialog.py @@ -39,7 +39,7 @@ ) from ._icon import app_icon -from ._widgets import NumericItem +from ._widgets import NumericItem, TearsDownOnClose, shutdown_worker_thread if sys.platform == "win32": @@ -161,7 +161,7 @@ def lessThan(self, left: QModelIndex, right: QModelIndex) -> bool: # noqa: N802 return left_item < right_item -class OpenProcessDialog(QDialog): +class OpenProcessDialog(TearsDownOnClose, QDialog): """Process picker. Returns the opened ``AbstractProcess`` via ``.process``.""" COL_PID = 0 @@ -289,7 +289,7 @@ def _populate_processes(self) -> None: worker = _ProcessListWorker(self) worker.rows_ready.connect(self._on_rows_ready) - worker.finished.connect(self._on_scan_finished) + worker.finished.connect(lambda w=worker: self._on_scan_finished(w)) self._scan_worker = worker worker.start() @@ -321,22 +321,24 @@ def _on_rows_ready(self, rows) -> None: self._table.selectRow(row) break - def _on_scan_finished(self) -> None: - worker = self._scan_worker - self._scan_worker = None - if worker is not None: - worker.deleteLater() + def _on_scan_finished(self, worker) -> None: + """Retire the enumeration that just finished — and only that one. + + ``finished`` is queued and can land after a refresh started a + replacement; clearing ``_scan_worker`` blindly would ``deleteLater()`` + that running worker, which aborts the process. + """ + if worker is self._scan_worker: + self._scan_worker = None + worker.deleteLater() - def closeEvent(self, event): # noqa: N802 — Qt naming + def _teardown(self) -> None: self._refresh_timer.stop() - if self._scan_worker is not None and self._scan_worker.isRunning(): - try: - self._scan_worker.rows_ready.disconnect() - self._scan_worker.finished.disconnect() - except (RuntimeError, TypeError): - pass - self._scan_worker.wait(1000) - super().closeEvent(event) + # Short join on purpose: this runs on the click that dismisses the + # picker, waiting for a process list nobody will read. A scan that + # outlasts it is detached rather than left parented to a dying dialog. + shutdown_worker_thread(self._scan_worker, wait_ms=250) + self._scan_worker = None def _on_filter_changed(self, text: str) -> None: self._proxy.setFilterFixedString(text) diff --git a/PyMemoryEditor/app/pointer_scan_dialog.py b/PyMemoryEditor/app/pointer_scan_dialog.py index 70db079..a51a6aa 100644 --- a/PyMemoryEditor/app/pointer_scan_dialog.py +++ b/PyMemoryEditor/app/pointer_scan_dialog.py @@ -55,6 +55,7 @@ from ._widgets import ( MONOSPACE_FAMILY, NumericItem, + TearsDownOnClose, parse_hex_address, shutdown_worker_thread, ) @@ -319,7 +320,7 @@ def _resolve_and_read( _COL_VALUE = 5 -class PointerScanDialog(QDialog): +class PointerScanDialog(TearsDownOnClose, QDialog): """Reverse pointer scan with a Cheat-Engine-style result table.""" # qulonglong: 64-bit addresses overflow Qt's signed-32-bit default. @@ -720,6 +721,9 @@ def _append_rows(self, rows: List[_ResultRow]) -> None: ) def _on_error(self, message: str) -> None: + # A metacall queued before _teardown disconnected still arrives. + if self._is_dismissed(): + return QMessageBox.critical(self, "Pointer Scan", message) self._count_label.setText("Scan failed.") @@ -997,10 +1001,9 @@ def _on_compare_saved(self) -> None: ) self._export_btn.setEnabled(True) - def closeEvent(self, event): # noqa: N802 — Qt naming + def _teardown(self) -> None: # 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) diff --git a/PyMemoryEditor/app/threads_dialog.py b/PyMemoryEditor/app/threads_dialog.py index 5c6694f..738e635 100644 --- a/PyMemoryEditor/app/threads_dialog.py +++ b/PyMemoryEditor/app/threads_dialog.py @@ -43,6 +43,7 @@ def __init__(self, process: AbstractProcess, parent=None): # self-throttles if an enumeration takes longer than the interval. super().__init__(process, refresh_interval_ms=300, parent=parent) self._threads: List[ThreadInfo] = [] + self._empty_streak = 0 # see _fetch_data; worker thread only self.setWindowTitle(f"Threads — PID {process.pid}") self.resize(640, 520) @@ -106,7 +107,22 @@ def _set_loading_hint(self) -> None: self._count_label.setText("Enumerating threads…") def _fetch_data(self): - return list(self._process.get_threads()) + threads = list(self._process.get_threads()) + # A live process always has a thread, but `get_threads` reports an + # exited one as an empty list on Linux/Windows (only macOS raises) — + # so the window would sit at "0 thread(s)" forever while Memory Map and + # Modules say the process is gone. Two in a row before saying so: an + # empty snapshot is legal-but-rare on a live process, and the first + # fetch is reported the moment it fails. + if not threads: + self._empty_streak += 1 + if self._empty_streak >= 2: + raise RuntimeError( + "The process reported no threads — it has most likely exited." + ) + return threads + self._empty_streak = 0 + return threads def _on_data_ready(self, threads) -> None: self._threads = list(threads) @@ -169,3 +185,9 @@ def _on_data_failed(self, message: str) -> None: QMessageBox.critical( self, "Threads", f"Failed to enumerate threads:\n\n{message}" ) + + def _on_polling_stopped(self) -> None: + self._count_label.setText( + "Failed to enumerate threads — auto-refresh stopped. " + "Close and reopen this window to retry." + ) diff --git a/PyMemoryEditor/process/abstract.py b/PyMemoryEditor/process/abstract.py index 35ff1b8..68a6df8 100644 --- a/PyMemoryEditor/process/abstract.py +++ b/PyMemoryEditor/process/abstract.py @@ -227,6 +227,16 @@ def get_threads(self) -> Generator[ThreadInfo, None, None]: The ``tid`` field's *meaning* is platform-specific (POSIX TID on Linux, DWORD TID on Windows, Mach port name on macOS). + A process that has exited is *not* reported uniformly: Linux and + Windows enumerate nothing and yield an empty sequence (their sources — + ``/proc//task`` and a system-wide thread snapshot filtered by pid — + simply come up empty), while macOS raises ``OSError`` because + ``task_threads`` fails on a dead task. Don't read "no threads" as "still + running": check liveness separately if that distinction matters. + Contrast :meth:`get_memory_regions` and :meth:`get_modules`, which do + raise :class:`~PyMemoryEditor.ProcessIDNotExistsError` on every + platform. + Use :attr:`main_thread` for the conventional "main thread" shortcut. """ raise NotImplementedError() diff --git a/tests/app/test_auto_refresh_dialog.py b/tests/app/test_auto_refresh_dialog.py new file mode 100644 index 0000000..67cab35 --- /dev/null +++ b/tests/app/test_auto_refresh_dialog.py @@ -0,0 +1,475 @@ +# -*- coding: utf-8 -*- + +""" +Tests for ``PyMemoryEditor/app/_auto_refresh_dialog.py``. + +Regression cover for issue #74: these dialogs poll every 300–1000 ms and used to +report every failed fetch with a modal. A modal runs a nested event loop, so the +timer kept firing underneath and the dialogs stacked — hundreds of them. + +The contract now: a failure is reported only when it changes what the user sees +— the first fetch (empty window) or the one that makes the poll give up (frozen +table) — and never twice for the same error. + +Skipped when ``PySide6`` isn't installed (the runtime dependency is opt-in via +the ``app`` extra). +""" + +import os +import time + +import pytest + + +pytest.importorskip("PySide6", reason="App tests require PySide6 (install with [app] extra).") + +# Offscreen platform plugin: no display server needed, runs on CI. +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + + +@pytest.fixture(scope="module") +def qapp(): + """A single QApplication for the module (Qt allows only one per process).""" + from PySide6.QtWidgets import QApplication + + return QApplication.instance() or QApplication([]) + + +def _spin(qapp, milliseconds: int) -> None: + """Pump the event loop for a while, so timers fire and workers land.""" + from PySide6.QtCore import QElapsedTimer + + elapsed = QElapsedTimer() + elapsed.start() + while elapsed.elapsed() < milliseconds: + qapp.processEvents() + time.sleep(0.005) + + +@pytest.fixture +def short_grace(monkeypatch): + """Shrink the give-up window so the timing tests stay sub-second.""" + from PyMemoryEditor.app import _auto_refresh_dialog + + monkeypatch.setattr(_auto_refresh_dialog, "_FAILURE_GRACE_MS", 150) + return 150 + + +@pytest.fixture(scope="module") +def dialog_parent(qapp): + """A hidden parent, so Qt owns the dialogs instead of Python's GC. + + A parentless QDialog dies with the test's reference, taking its worker + children with it while the event queue may still hold events for them — + which segfaulted a *later* test's ``qtbot.wait``. Never deleted on purpose: + a teardown here would recreate that window. + """ + from PySide6.QtWidgets import QWidget + + return QWidget() + + +@pytest.fixture +def make_dialog(qapp, dialog_parent): + """Factory for an ``AutoRefreshTableDialog`` whose fetch can be made to fail. + + Closed at the end of each test (that exercises the teardown) but owned by + ``dialog_parent``, not destroyed mid-session. + """ + from PyMemoryEditor.app._auto_refresh_dialog import AutoRefreshTableDialog + + class _Dialog(AutoRefreshTableDialog): + def __init__(self, interval_ms, failing): + super().__init__( + object(), refresh_interval_ms=interval_ms, parent=dialog_parent + ) + self.failing = failing + self.flapping = False + self.fetches = 0 + self.errors = [] + self.rendered = 0 + self.gave_up = 0 + + def _fetch_data(self): + # Runs on the worker thread, like the real enumerations. + self.fetches += 1 + if self.flapping: + self.failing = not self.failing + if self.failing: + raise RuntimeError('The process ID "4242" does not exist.') + return ["ok"] + + def _on_data_ready(self, data): + self.rendered += 1 + + def _on_data_failed(self, message): + # Stands in for the subclasses' QMessageBox.critical. + self.errors.append(message) + + def _on_polling_stopped(self): + # Stands in for the subclasses' "auto-refresh stopped" label. + self.gave_up += 1 + + dialogs = [] + + def _factory(interval_ms=50, failing=False): + dialog = _Dialog(interval_ms, failing) + dialog.refresh() + dialog._start_auto_refresh() + _spin(qapp, 100) # let the first fetch land + dialogs.append(dialog) + return dialog + + yield _factory + + for dialog in dialogs: + dialog.close() + _spin(qapp, 50) + + +def test_a_target_that_dies_is_reported_once(qapp, short_grace, make_dialog): + """A target that fails on every tick must produce a single error report.""" + dialog = make_dialog() + assert dialog.errors == [] + assert dialog.rendered > 0 + + dialog.failing = True + _spin(qapp, 600) + + # Without the fix this grew by one modal per 50 ms tick — and because each + # modal runs a nested event loop, the timer kept firing and they stacked. + assert len(dialog.errors) == 1 + assert "does not exist" in dialog.errors[0] + + +def test_polling_stops_when_failures_persist(qapp, short_grace, make_dialog): + """A dead target isn't coming back under the same handle — stop polling.""" + dialog = make_dialog() + dialog.failing = True + _spin(qapp, 600) + + assert dialog._auto_timer is not None and not dialog._auto_timer.isActive() + fetches = dialog.fetches + _spin(qapp, 300) + # Bounded work: it gives up instead of burning a worker per tick forever. + assert dialog.fetches == fetches + # And it says so exactly once, so the frozen table isn't silently stale. + assert dialog.gave_up == 1 + + +def test_a_flapping_target_never_stacks_dialogs(qapp, make_dialog): + """Fail / succeed / fail / succeed must stay silent, not pop a modal a tick. + + Reporting every failure looked safe because the same error is never reported + twice — but each success cleared that latch, so an alternating target popped + a modal per failure, nesting event loops until the stack blew up. + """ + dialog = make_dialog() + dialog.flapping = True + _spin(qapp, 800) + + assert dialog.errors == [] + assert dialog.gave_up == 0 + assert dialog._auto_timer.isActive() # a healing target keeps polling + assert dialog.rendered > 1 # and keeps rendering its good ticks + + +def test_the_first_fetch_failure_is_reported_immediately(qapp, make_dialog): + """Nothing to look at but the error, so don't sit on it for the grace window.""" + dialog = make_dialog(failing=True) # broken before the window ever filled + + assert len(dialog.errors) == 1 + assert dialog.gave_up == 0 # reported on its own merit, not by giving up + assert dialog._auto_timer.isActive() + + +def test_failures_inside_the_grace_window_are_not_reported(make_dialog): + """A blip (a snapshot that fails while the target loads images) stays quiet. + + Driven through the handlers: racing a 50 ms poll to land an exact number of + failures would only make the test flaky. + """ + dialog = make_dialog() + + for _ in range(5): + dialog._handle_failed("transient") + assert dialog.errors == [] + assert dialog.gave_up == 0 + assert dialog._auto_timer.isActive() + + dialog._handle_ready(["ok"]) # healed + assert dialog._failing_since is None # the streak clock resets + + +def test_explicit_refresh_re_arms_the_poll_and_the_error_latch( + qapp, short_grace, make_dialog +): + """Reopening the dialog (the main window calls ``refresh()``) resumes it.""" + dialog = make_dialog() + dialog.failing = True + _spin(qapp, 600) + assert len(dialog.errors) == 1 + assert not dialog._auto_timer.isActive() + + dialog.failing = False + dialog.refresh() + _spin(qapp, 200) + assert dialog._auto_timer.isActive() + assert len(dialog.errors) == 1 + + # A later give-up is reported again — the latch was cleared, not sealed. + dialog.failing = True + _spin(qapp, 600) + assert len(dialog.errors) == 2 + assert dialog.gave_up == 2 + + +def test_the_close_button_tears_the_dialog_down(qapp, make_dialog): + """The Close button calls accept(), which delivers no QCloseEvent. + + The teardown used to live in ``closeEvent`` alone, so dismissing a polling + dialog the normal way left it hidden and *still polling*, leaking a thread + per open/close cycle. + """ + dialog = make_dialog() + assert dialog._auto_timer.isActive() + fetches = dialog.fetches + + dialog.accept() # exactly what the Close button is wired to + _spin(qapp, 300) + + assert not dialog.isVisible() + assert not dialog._auto_timer.isActive() + assert dialog.fetches == fetches + assert dialog._worker is None + + +def test_escape_tears_the_dialog_down_too(qapp, make_dialog): + """Esc reaches reject(), the other half of done(). + + A real key event, not a ``reject()`` call: Esc is handled inside ``QDialog`` + on the C++ side, so this proves the override is reached through Qt's virtual + dispatch. + """ + from PySide6.QtCore import Qt + from PySide6.QtTest import QTest + + dialog = make_dialog() + dialog.show() + _spin(qapp, 50) + fetches = dialog.fetches + + QTest.keyClick(dialog, Qt.Key_Escape) + _spin(qapp, 300) + + assert not dialog.isVisible() + assert not dialog._auto_timer.isActive() + assert dialog.fetches == fetches + assert dialog._worker is None + + +def test_teardown_runs_once_however_the_dialog_is_dismissed(qapp, make_dialog): + """close() → closeEvent → reject() → done() must not tear down twice.""" + dialog = make_dialog() + calls = [] + real_teardown = dialog._teardown + dialog._teardown = lambda: (calls.append(True), real_teardown())[1] + + dialog.close() + _spin(qapp, 100) + dialog.accept() # a second dismissal must be a no-op + + assert calls == [True] + + +def test_a_late_finished_does_not_retire_the_worker_that_replaced_it(make_dialog): + """``finished`` is queued and can land after the next tick started a fetch. + + Retiring ``self._worker`` blindly dropped the reference to the *running* + replacement and scheduled it for deletion — that aborts the process. + """ + dialog = make_dialog() + dialog._start_fetch() + current = dialog._worker + assert current is not None + + class _Stale: + def __init__(self): + self.deleted = False + + def deleteLater(self): + self.deleted = True + + stale = _Stale() + dialog._on_worker_finished(stale) + + assert dialog._worker is current # still tracked, still joinable + assert stale.deleted # and the one that really finished is retired + + +def test_nothing_fetches_after_the_dialog_was_torn_down(qapp, make_dialog): + """The teardown latch is one-way, so a fetch started later is unjoinable.""" + dialog = make_dialog() + dialog.accept() + _spin(qapp, 100) + fetches = dialog.fetches + + dialog.refresh() # a stale caller poking a dismissed dialog + _spin(qapp, 200) + + assert dialog.fetches == fetches + assert dialog._worker is None + assert not dialog._auto_timer.isActive() + + +def test_threads_dialog_treats_an_empty_enumeration_as_a_dead_target( + qapp, short_grace, dialog_parent +): + """Linux and Windows report an exited process as *no threads*, not an error, + so the window would sit at "0 thread(s)" forever.""" + from PyMemoryEditor.app.threads_dialog import ThreadsDialog + from PyMemoryEditor.process.thread_info import ThreadInfo + + alive = [ThreadInfo(tid=1, start_address=None, state="S", priority=20, raw="1")] + state = {"threads": alive} + + class _Process: + pid = 4242 + + def get_threads(self): + return iter(state["threads"]) + + reported = [] + dialog = ThreadsDialog(_Process(), parent=dialog_parent) + dialog._on_data_failed = lambda message: reported.append(message) + try: + _spin(qapp, 400) + assert dialog._model.rowCount() == 1 + assert reported == [] + + state["threads"] = [] # the target exits + # 300 ms poll: one tick to see the empty list, another to call it, and + # the grace window on top before it is worth telling the user. + _spin(qapp, 1400) + + assert len(reported) == 1 + assert "exited" in reported[0] + finally: + dialog.close() + _spin(qapp, 50) + + +def test_threads_dialog_tolerates_a_single_empty_enumeration(qapp, dialog_parent): + """One empty snapshot is not proof of death: ``Thread32First`` can come up + dry on a live process, and the first fetch is reported immediately.""" + from PyMemoryEditor.app.threads_dialog import ThreadsDialog + from PyMemoryEditor.process.thread_info import ThreadInfo + + alive = [ThreadInfo(tid=1, start_address=None, state="S", priority=20, raw="1")] + state = {"threads": [], "calls": 0} + + class _Process: + pid = 4242 + + def get_threads(self): + state["calls"] += 1 + # Empty once, then healthy again. + return iter(state["threads"] if state["calls"] > 1 else []) + + reported = [] + dialog = ThreadsDialog(_Process(), parent=dialog_parent) + dialog._on_data_failed = lambda message: reported.append(message) + try: + state["threads"] = alive + _spin(qapp, 500) + + assert reported == [] # the blip healed, nobody was told anything + assert dialog._model.rowCount() == 1 + assert dialog._auto_timer.isActive() + finally: + dialog.close() + _spin(qapp, 50) + + +def test_hex_viewer_stops_polling_and_logs_once_on_a_dead_target( + qapp, dialog_parent, caplog, monkeypatch +): + """The Hex Viewer is the other polling window, and it never gave up. + + Against a dead target every tick logged a warning — up to 20 a second at the + 50 ms floor, straight into the Log Console. + """ + import logging + + from PyMemoryEditor.app import memory_viewer_dialog as mv + + monkeypatch.setattr(mv, "_FAILURE_GRACE_MS", 150) + + class _Process: + pid = 4242 + + def read_process_memory(self, address, pytype, length): + raise OSError("the process is gone") + + viewer = mv.MemoryViewerDialog( + _Process(), address=0x1000, length=64, parent=dialog_parent + ) + try: + viewer._interval_spin.setValue(50) + with caplog.at_level(logging.WARNING, logger="PyMemoryEditor"): + viewer._auto_btn.setChecked(True) # the user opts into polling + _spin(qapp, 600) + + warnings = [r for r in caplog.records if "Hex viewer read failed" in r.message] + assert len(warnings) == 1 # once per streak, not once per tick + assert not viewer._timer.isActive() + assert not viewer._auto_btn.isChecked() # and the button says so + assert "auto-refresh stopped" in viewer._status.text() + finally: + viewer.close() + _spin(qapp, 50) + + +def test_shutdown_really_disconnects_the_worker(qapp, dialog_parent, make_dialog): + """`worker.disconnect()` is a no-op in PySide6 — it raises TypeError. + + `shutdown_worker_thread` swallowed that, so nothing was ever disconnected. + The four-argument static form is the one that works for slots that are bound + methods of the owning dialog. + """ + from PyMemoryEditor.app._auto_refresh_dialog import _DataWorker + from PyMemoryEditor.app._widgets import shutdown_worker_thread + + dialog = make_dialog() + # Quiesce the dialog first: with its own poll running, a result already in + # flight would land during the pump below and move the counters for reasons + # that have nothing to do with this test. + dialog._auto_timer.stop() + _spin(qapp, 100) + rendered, errors = dialog.rendered, len(dialog.errors) + + worker = _DataWorker(lambda: ["x"], dialog) + worker.ready.connect(dialog._handle_ready) + worker.failed.connect(dialog._handle_failed) + + shutdown_worker_thread(worker, wait_ms=0) + worker.ready.emit(["late"]) + worker.failed.emit("late failure") + qapp.processEvents() + + assert dialog.rendered == rendered + assert len(dialog.errors) == errors + + +def test_a_dismissed_dialog_reports_nothing(qapp, make_dialog): + """The same hazard from the receiving end: a delivery already queued when + the teardown ran must not pop a modal over a window that is gone.""" + dialog = make_dialog() + dialog.accept() + _spin(qapp, 50) + + dialog._handle_failed("late failure") + dialog._handle_ready(["late"]) + + assert dialog.errors == [] + assert dialog.gave_up == 0