Block on the window itself rather than the application's event loop

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`_.

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()

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

mne/viz/backends/tests/test_utils.py

@@ -2,15 +2,25 @@
 # 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
 
 from mne import create_info
 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)

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):

pyproject.toml

@@ -324,7 +324,7 @@ addopts = [
   "--ignore=mne/report/js_and_css",
   "--junit-xml=junit-results.xml",
   "--tb=short",
-  "-rfEXs",
+  "-ra",
 ]
 testpaths = [
   "mne",

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

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

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}"

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