From 52ac3800054815b71d2e52ebf7d7a894d7d424f2 Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Tue, 18 Aug 2026 18:41:32 -0400 Subject: [PATCH 1/2] Block on the window itself rather than the application's event loop [ci skip] --- mne/viz/backends/_qt.py | 4 +- mne/viz/backends/_utils.py | 75 ++++++++++++++++++++++++++++++++++++++ mne/viz/utils.py | 5 +-- 3 files changed, 79 insertions(+), 5 deletions(-) diff --git a/mne/viz/backends/_qt.py b/mne/viz/backends/_qt.py index a0ae27387ba..0792540d633 100644 --- a/mne/viz/backends/_qt.py +++ b/mne/viz/backends/_qt.py @@ -112,7 +112,7 @@ from ._utils import ( _ICONS_PATH, _init_mne_qtapp, - _qt_app_exec, + _qt_block, _qt_detect_theme, _qt_disable_paint, _qt_get_stylesheet, @@ -736,7 +736,7 @@ def _show(self, block=False): _qt_raise_window(self) _Widget._show(self) if block: - _qt_app_exec(self._app) + _qt_block(self) def _close(self): self.close() diff --git a/mne/viz/backends/_utils.py b/mne/viz/backends/_utils.py index 8201a2a7a7c..fb0d8e7798d 100644 --- a/mne/viz/backends/_utils.py +++ b/mne/viz/backends/_utils.py @@ -4,10 +4,12 @@ # Copyright the MNE-Python contributors. import collections.abc +import contextlib import functools import os import platform import signal +import socket import sys from contextlib import contextmanager from ctypes import c_char_p, c_void_p, cdll @@ -275,6 +277,79 @@ def _qt_app_exec(app): signal.signal(signal.SIGINT, old_signal) +@contextmanager +def _allow_qt_interrupt(loop): + """Let SIGINT out of a Qt event loop, which otherwise never lets Python run. + + Adapted from Matplotlib: a socketpair registered as the signal wakeup fd makes Qt + wake up on delivery, and running any Python at all (the notifier callback) is what + lets the interpreter reach the handler. + """ + from qtpy.QtCore import QSocketNotifier + + old_handler = signal.getsignal(signal.SIGINT) + if old_handler in (None, signal.SIG_IGN, signal.SIG_DFL): + yield # a non-Python handler owns SIGINT; don't get in its way + return + wsock, rsock = socket.socketpair() + wsock.setblocking(False) + rsock.setblocking(False) + old_wakeup_fd = signal.set_wakeup_fd(wsock.fileno()) + notifier = QSocketNotifier(rsock.fileno(), QSocketNotifier.Type.Read) + + def _drain(): + with contextlib.suppress(BlockingIOError): + rsock.recv(1) # re-arm the notifier, which the wakeup write triggered + + notifier.activated.connect(_drain) + handler_args = [] + signal.signal(signal.SIGINT, lambda *args: (handler_args.append(args), loop.quit())) + try: + yield + finally: + notifier.setEnabled(False) + signal.set_wakeup_fd(old_wakeup_fd) + signal.signal(signal.SIGINT, old_handler) + wsock.close() + rsock.close() + # Hand the signal to whoever owned it (a notebook kernel, IPython, plain + # Python) with the frame it arrived on, so it is reported their way + if handler_args: + old_handler(*handler_args[0]) + + +def _qt_block(window): + """Block until ``window`` is closed, keeping it interactive. + + Unlike :func:`_qt_app_exec` this runs a nested loop instead of the application's + own, so it neither quits the app nor stops an event loop something else owns (a + notebook kernel, an IDE), and it returns when this window closes rather than when + the last one does. + """ + from qtpy.QtCore import QEvent, QEventLoop, QObject + + if not window.isVisible(): + return + + loop = QEventLoop() + + class _CloseWatcher(QObject): + def eventFilter(self, obj, event): + if event.type() == QEvent.Type.Close: + loop.quit() + return False + + watcher = _CloseWatcher() + window.installEventFilter(watcher) + window.destroyed.connect(loop.quit) # closed without a Close event + try: + with _allow_qt_interrupt(loop): + loop.exec() + finally: + with contextlib.suppress(RuntimeError): # window may already be deleted + window.removeEventFilter(watcher) + + def _qt_detect_theme(): try: import darkdetect diff --git a/mne/viz/utils.py b/mne/viz/utils.py index 3ca3abaac8f..7e0b48f7e7e 100644 --- a/mne/viz/utils.py +++ b/mne/viz/utils.py @@ -189,9 +189,8 @@ def _show_browser(show=True, block=True, fig=None, **kwargs): plt_show(show, block=block, **kwargs) else: from qtpy.QtCore import Qt - from qtpy.QtWidgets import QApplication - from .backends._utils import _qt_app_exec + from .backends._utils import _qt_block if fig is not None and os.getenv("_MNE_BROWSER_BACK", "").lower() == "true": fig.setWindowFlags(fig.windowFlags() | Qt.WindowStaysOnBottomHint) @@ -200,7 +199,7 @@ def _show_browser(show=True, block=True, fig=None, **kwargs): # If block=False, a Qt-Event-Loop has to be started # somewhere else in the calling code. if block: - _qt_app_exec(QApplication.instance()) + _qt_block(fig) def _check_delayed_ssp(container): From 62ecf7198e14625f200ce77ee694b98a2ff4f229 Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Thu, 20 Aug 2026 11:14:50 -0400 Subject: [PATCH 2/2] Test _qt_block, including the interrupt path, enforce more skip --- doc/changes/dev/14179.bugfix.rst | 1 + mne/viz/backends/tests/test_utils.py | 65 +++++++++++++++++++++++++++ pyproject.toml | 2 +- tools/github_actions_env_vars.sh | 8 ++-- tools/github_actions_test.sh | 9 +++- tools/github_actions_verify_python.sh | 10 ++--- tools/vulture_allowlist.py | 4 ++ 7 files changed, 89 insertions(+), 10 deletions(-) create mode 100644 doc/changes/dev/14179.bugfix.rst diff --git a/doc/changes/dev/14179.bugfix.rst b/doc/changes/dev/14179.bugfix.rst new file mode 100644 index 00000000000..94dcf3ab680 --- /dev/null +++ b/doc/changes/dev/14179.bugfix.rst @@ -0,0 +1 @@ +Fixed a bug where ``block=True`` in plotting functions such as :meth:`mne.io.Raw.plot` ran the Qt application's own event loop, which deadlocked or silently did not block when something else already owned that loop (for example inside a notebook kernel), and could not be interrupted, by `Eric Larson`_. diff --git a/mne/viz/backends/tests/test_utils.py b/mne/viz/backends/tests/test_utils.py index 2cf949829e4..cabdccb6b62 100644 --- a/mne/viz/backends/tests/test_utils.py +++ b/mne/viz/backends/tests/test_utils.py @@ -2,6 +2,13 @@ # License: BSD-3-Clause # Copyright the MNE-Python contributors. +import os +import signal +import subprocess +import sys +import threading +import time + import numpy as np import pytest @@ -9,8 +16,11 @@ from mne.io import RawArray from mne.viz.backends._utils import ( _check_color, + _display_is_valid, _get_colormap_from_array, + _init_mne_qtapp, _pixmap_to_ndarray, + _qt_block, _qt_is_dark, ) from mne.viz.utils import _is_dark @@ -80,3 +90,58 @@ def test_theme_colors(pg_backend, theme, monkeypatch, tmp_path): for widget in (fig.mne.toolbar, fig.statusBar()): _assert_correct_darkness(widget, is_dark) + + +def test_qt_block(qtbot): + """Test that _qt_block waits for its own window and nothing else.""" + pytest.importorskip("qtpy") # pytest-qt can be installed without a Qt binding + from qtpy.QtCore import QTimer + from qtpy.QtWidgets import QWidget + + win, other = QWidget(), QWidget() + for widget in (win, other): + qtbot.addWidget(widget) + widget.show() + QTimer.singleShot(300, win.close) + t0 = time.time() + _qt_block(win) + elapsed = time.time() - t0 + assert 0.1 < elapsed < 10, elapsed + assert not win.isVisible() + assert other.isVisible() # blocking is per-window, not until the last one closes + _qt_block(win) # a closed window returns immediately rather than hanging + other.close() + + +# Adapted from Matplotlib's test_backends_interactive.py::test_sigint: the scenario runs +# in a subprocess because an in-process SIGINT fights pytest's own handling, and because +# a loop that fails to wake up hangs until the subprocess timeout, not the whole suite. +def _sigint_impl(): + from qtpy.QtWidgets import QWidget + + app = _init_mne_qtapp() # keep a reference, PyQt6 garbage collects it otherwise + win = QWidget() + win.show() + # A Qt event loop keeps the interpreter from running, so this only arrives if + # _qt_block wakes Qt up on delivery + threading.Timer(1.0, lambda: os.kill(os.getpid(), signal.SIGINT)).start() + try: + _qt_block(win) + except KeyboardInterrupt: + print(f"SUCCESS still_open={win.isVisible()}", flush=True) + app.closeAllWindows() + + +@pytest.mark.skipif(sys.platform == "win32", reason="Cannot send SIGINT on Windows") +def test_qt_block_sigint(): + """Test that a blocked window can be interrupted.""" + pytest.importorskip("qtpy") + if not _display_is_valid(): + pytest.skip("Requires a valid display") + # pytest imports this file as a top-level module, so name the installed path + code = "from mne.viz.backends.tests.test_utils import _sigint_impl; _sigint_impl()" + # A hang here means the signal never reached Python, which is the bug this guards + proc = subprocess.run( + [sys.executable, "-c", code], capture_output=True, text=True, timeout=60 + ) + assert "SUCCESS still_open=True" in proc.stdout, (proc.stdout, proc.stderr) diff --git a/pyproject.toml b/pyproject.toml index f2d17cf1967..1d1529c88a5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -324,7 +324,7 @@ addopts = [ "--ignore=mne/report/js_and_css", "--junit-xml=junit-results.xml", "--tb=short", - "-rfEXs", + "-ra", ] testpaths = [ "mne", diff --git a/tools/github_actions_env_vars.sh b/tools/github_actions_env_vars.sh index 4ee5591e323..a687363cd6f 100755 --- a/tools/github_actions_env_vars.sh +++ b/tools/github_actions_env_vars.sh @@ -44,16 +44,18 @@ if [[ "$MNE_CI_KIND" == "pip"* ]]; then fi echo "MNE_QT_BACKEND=PySide6" | tee -a $GITHUB_ENV elif [[ "$MNE_CI_KIND" == "pip-ft" ]]; then - echo "No env vars to set" + echo "MNE_TEST_ALLOW_SKIP=.*(Requires (spm|brainstorm|misc|testing) dataset|Requires (MNE-C|FreeSurfer)|MNE_SKIP_NETWORK_TESTS|could not import|No module named|not installed|not available|has __version__|needs >=|[Nn]eeds [a-z0-9_.-]+|[Rr]equires [a-z0-9_.-]+$|[A-Za-z0-9_.-]+ (is )?required|fixed by [a-z0-9_.-]+ [0-9]|CUDA not|Numba not|SCIPY_ARRAY_API|[Aa]rray API|PySide6 causes segfaults).*" | tee -a $GITHUB_ENV else - echo "✕ ERROR: Unrecognized MNE_CI_KIND=${MNE_CI_KIND}" + echo "::error::Unrecognized MNE_CI_KIND=${MNE_CI_KIND}" exit 1 fi elif [[ "$MNE_CI_KIND" == "minimal" ]]; then + echo "MNE_TEST_ALLOW_SKIP=.*(Requires (spm|brainstorm|misc|testing) dataset|Requires (MNE-C|FreeSurfer)|MNE_SKIP_NETWORK_TESTS|could not import|No module named|not installed|not available|has __version__|needs >=|[Nn]eeds [a-z0-9_.-]+|[Rr]equires [a-z0-9_.-]+$|[A-Za-z0-9_.-]+ (is )?required|fixed by [a-z0-9_.-]+ [0-9]|CUDA not|Numba not|SCIPY_ARRAY_API|[Aa]rray API|PySide6 causes segfaults).*" | tee -a $GITHUB_ENV echo "MNE_QT_BACKEND=PySide6" | tee -a $GITHUB_ENV elif [[ "$MNE_CI_KIND" == "old" ]]; then echo "MNE_IGNORE_WARNINGS_IN_TESTS=true" | tee -a $GITHUB_ENV echo "MNE_SKIP_NETWORK_TESTS=1" | tee -a $GITHUB_ENV + echo "MNE_TEST_ALLOW_SKIP=.*(Requires (spm|brainstorm|misc|testing) dataset|Requires (MNE-C|FreeSurfer)|MNE_SKIP_NETWORK_TESTS|could not import|No module named|not installed|not available|has __version__|needs >=|[Nn]eeds [a-z0-9_.-]+|[Rr]equires [a-z0-9_.-]+$|[A-Za-z0-9_.-]+ (is )?required|fixed by [a-z0-9_.-]+ [0-9]|CUDA not|Numba not|SCIPY_ARRAY_API|[Aa]rray API|PySide6 causes segfaults).*" | tee -a $GITHUB_ENV echo "MNE_QT_BACKEND=PyQt6" | tee -a $GITHUB_ENV elif [[ "$MNE_CI_KIND" == "conda" ]]; then echo "Setting conda env vars for $MNE_CI_KIND" @@ -62,7 +64,7 @@ elif [[ "$MNE_CI_KIND" == "conda" ]]; then echo "MNE_TEST_ALLOW_SKIP=.*(on conda|Requires (spm|brainstorm|misc) dataset|CUDA not|Flakey verbose behavior|PySide6 causes segfaults|SCIPY_ARRAY_API).*" | tee -a $GITHUB_ENV echo "MNE_QT_BACKEND=PySide6" | tee -a $GITHUB_ENV else - echo "✕ ERROR: Unrecognized MNE_CI_KIND=${MNE_CI_KIND}" + echo "::error::Unrecognized MNE_CI_KIND=${MNE_CI_KIND}" exit 1 fi if [[ "$CI_OS_NAME" == "windows"* ]]; then diff --git a/tools/github_actions_test.sh b/tools/github_actions_test.sh index 8a84e2b7ef8..a906960b538 100755 --- a/tools/github_actions_test.sh +++ b/tools/github_actions_test.sh @@ -2,6 +2,13 @@ set -eo pipefail +# An unset value disables the check in conftest entirely, so every job has to say what it +# is allowed to skip rather than silently skipping anything +test -n "${MNE_TEST_ALLOW_SKIP}" || { + echo "::error::MNE_TEST_ALLOW_SKIP is unset, so skips would go untracked" + exit 1 +} + if [[ "${CI_OS_NAME}" == "ubuntu"* ]]; then CONDITION="not (ultraslowtest or pgtest)" elif [[ "${CI_OS_NAME}" == "macos"* ]]; then @@ -14,7 +21,7 @@ elif [[ "${CI_OS_NAME}" == "macos"* ]]; then elif [[ "${CI_OS_NAME}" == "windows"* ]]; then CONDITION="not (slowtest or pgtest)" else - echo "✕ ERROR: Unrecognized CI_OS_NAME=${CI_OS_NAME}" + echo "::error::Unrecognized CI_OS_NAME=${CI_OS_NAME}" exit 1 fi if [ "${MNE_CI_KIND}" == "notebook" ]; then diff --git a/tools/github_actions_verify_python.sh b/tools/github_actions_verify_python.sh index 4fae44a2ca5..01562e3b0cc 100755 --- a/tools/github_actions_verify_python.sh +++ b/tools/github_actions_verify_python.sh @@ -10,7 +10,7 @@ else fi WANT_PYTHON_VERSION=$(echo "$WANT_PYTHON_VERSION" | sed 's/t$//g') if [[ -z "$WANT_PYTHON_VERSION" ]]; then - echo "✕ ERROR: Missing required argument: want Python version (e.g., 3.11)" + echo "::error::Missing required argument: want Python version (e.g., 3.11)" exit 1 fi @@ -30,24 +30,24 @@ elif [[ "${MNE_CI_KIND}" == "old" ]]; then elif [[ "${MNE_CI_KIND}" == 'pip'* ]] || [[ "${MNE_CI_KIND}" == "minimal" ]]; then WANT="/hostedtoolcache/" else - echo "✕ ERROR: Unrecognized MNE_CI_KIND=${MNE_CI_KIND}" + echo "::error::Unrecognized MNE_CI_KIND=${MNE_CI_KIND}" exit 1 fi if [[ "${GOT_PYTHON}" != *"${WANT}"* ]]; then - echo "✕ ERROR: Did not find \"${WANT}\" from PATH:" + echo "::error::Did not find \"${WANT}\" in PATH (dumped below)" tr ':' '\n' <<< "$PATH" exit 1 else echo "☑ Found expected \"${WANT}\"" fi if [[ "${GOT_PYTHON_VERSION}" != *"${WANT_PYTHON_VERSION}"* ]]; then - echo "✕ ERROR: Did not find expected Python version \"${WANT_PYTHON_VERSION}\"" + echo "::error::Did not find expected Python version \"${WANT_PYTHON_VERSION}\"" exit 1 else echo "☑ Found expected Python version \"${WANT_PYTHON_VERSION}\"" fi if [[ "${GOT_FREETHREADED}" != "${WANT_FREETHREADED}" ]]; then - echo "✕ ERROR: Expected free-threaded=${WANT_FREETHREADED} but got ${GOT_FREETHREADED}" + echo "::error::Expected free-threaded=${WANT_FREETHREADED} but got ${GOT_FREETHREADED}" exit 1 else echo "☑ Found expected free-threaded=${WANT_FREETHREADED}" diff --git a/tools/vulture_allowlist.py b/tools/vulture_allowlist.py index b54a2a6a02f..8b72ed27282 100644 --- a/tools/vulture_allowlist.py +++ b/tools/vulture_allowlist.py @@ -174,3 +174,7 @@ _qt_disable_paint _qt_get_stylesheet _show_help_fig + +# Called by Qt, or only from a subprocess (mne/viz/backends/tests/test_utils.py) +eventFilter +_sigint_impl