diff --git a/sources/editor/ui/qetelementeditor.cpp b/sources/editor/ui/qetelementeditor.cpp index 2453f04f0..9ae36def3 100644 --- a/sources/editor/ui/qetelementeditor.cpp +++ b/sources/editor/ui/qetelementeditor.cpp @@ -23,6 +23,7 @@ #include "../elementview.h" #include "../../qetmessagebox.h" #include "../../qetapp.h" +#include "../../qetmainwindow.h" #include "../../recentfiles.h" #include "../graphicspart/customelementpart.h" #include "../elementitemeditor.h" @@ -887,6 +888,13 @@ void QETElementEditor::openElement(const QString &filepath) */ void QETElementEditor::closeEvent(QCloseEvent *qce) { + //This editor is a plain QMainWindow, not a QETMainWindow, so the + //guard QETMainWindow::event() applies to the other editors is + //applied here instead -- before canClose(), which itself opens a + //modal dialog. + if (QETMainWindow::refuseCloseWhileModal(qce)) { + return; + } if (canClose()) { writeSettings(); setAttribute(Qt::WA_DeleteOnClose); diff --git a/sources/qetmainwindow.cpp b/sources/qetmainwindow.cpp index efc9f0e83..b3553d9d6 100644 --- a/sources/qetmainwindow.cpp +++ b/sources/qetmainwindow.cpp @@ -16,6 +16,7 @@ along with QElectroTech. If not, see . */ #include +#include #include #include #include @@ -290,6 +291,9 @@ void QETMainWindow::activateMenuBar() { } bool QETMainWindow::event(QEvent *e) { + if (e -> type() == QEvent::Close && refuseCloseWhileModal(e)) { + return(true); + } if (e -> type() == QEvent::WindowStateChange) { updateFullScreenAction(); } else if (first_activation_ && e -> type() == QEvent::WindowActivate) { @@ -299,6 +303,44 @@ bool QETMainWindow::event(QEvent *e) { return(QMainWindow::event(e)); } +/** + @brief QETMainWindow::refuseCloseWhileModal + Refuse to close an editor window while any modal dialog is running. + + A modal dialog's exec() runs a nested event loop. If a window is closed + during it, the window's WA_DeleteOnClose turns into a deleteLater() that + the *nested* loop processes: the window is destroyed while code that + belongs to it -- often the very function that opened the dialog -- is + still on the stack. Most of QET's dialogs are stack objects parented to + the window (BackupDialog, and every QET::QetMessageBox), so ~QWidget() + then deletes a stack object and the process aborts (issue #904). Even a + dialog without a parent would only trade that abort for a silent + use-after-free in the caller. + + Qt already ignores window-manager close requests for a window blocked by + a modal, so this is only reachable through close() called directly: the + File > Quit action, which macOS moves into the application menu where it + stays usable during a modal, and QETApp::quitQET() from the system tray. + + Handled in event(), before closeEvent() runs, because the editors' + closeEvent() starts closing projects before it decides whether to accept. + The dialog is raised so a refused quit is not silent. + + @param e : the QEvent::Close being delivered + @return true if the close was refused and must not be processed further +*/ +bool QETMainWindow::refuseCloseWhileModal(QEvent *e) +{ + QWidget *modal = QApplication::activeModalWidget(); + if (!modal) { + return(false); + } + modal -> raise(); + modal -> activateWindow(); + e -> ignore(); + return(true); +} + /** Base implementation of firstActivation (does nothing). */ diff --git a/sources/qetmainwindow.h b/sources/qetmainwindow.h index 3d90cee4a..9ad1862f0 100644 --- a/sources/qetmainwindow.h +++ b/sources/qetmainwindow.h @@ -30,6 +30,8 @@ class QETMainWindow : public QMainWindow { public: QETMainWindow(QWidget * = nullptr, Qt::WindowFlags = Qt::Widget); ~QETMainWindow() override; + + static bool refuseCloseWhileModal(QEvent *e); // methods protected: diff --git a/tests/modal-quit-regression/README.md b/tests/modal-quit-regression/README.md new file mode 100644 index 000000000..7ed261982 --- /dev/null +++ b/tests/modal-quit-regression/README.md @@ -0,0 +1,42 @@ +# Quit-during-modal regression test + +Guards the abort reported in #904. + +`QETDiagramEditor::openAndAddProject()` shows `BackupDialog` as a stack object +parented to the editor and `exec()`s it, and `QET::QetMessageBox` does the same +for every message box. `exec()` runs a nested event loop. Closing the editor +during that loop turns `WA_DeleteOnClose` into a `deleteLater()` that the nested +loop processes, so `~QWidget()` deletes the stack-allocated dialog and the +process aborts. `QETMainWindow::refuseCloseWhileModal()` refuses such a close. + +## Running it + +```bash +tests/modal-quit-regression/run.sh --binary build/qelectrotech +``` + +Needs `gdb` with Python. It runs on the offscreen platform, so no X server or +window manager is required. Takes about ten seconds. + +Exit codes: `0` survived, `1` crashed, `2` the scenario did not happen. + +## How it works, and why this way + +On Linux there is nothing to click: the menu bar belongs to the window the +dialog blocks, and Qt ignores window-manager close requests for a blocked +window. The reported route is macOS, where File > Quit lives in the application +menu and stays usable during a modal — and what it does is call `close()` while +the dialog's loop is running. The test does the same thing through gdb: + +1. break on `QDialog::exec()`; +2. let the dialog's loop run, then interrupt it, so the main thread is inside + the nested loop — the only place the bug exists, because a `deleteLater()` + posted *before* `exec()` started is not processed by that loop; +3. call `QETApp::quitQET()`, which closes every editor; +4. let it run: an unfixed build aborts within a second. + +It matches no window titles and no window ids. Titles are translated, and +`tests/ipc-regression` once shipped a pass that could not fail because of that. + +Checked both ways before being committed: it fails on a build without the fix +(signal 6, `free(): invalid size`) and passes with it. diff --git a/tests/modal-quit-regression/run.sh b/tests/modal-quit-regression/run.sh new file mode 100755 index 000000000..9ca956c26 --- /dev/null +++ b/tests/modal-quit-regression/run.sh @@ -0,0 +1,153 @@ +#!/usr/bin/env bash +# +# Quit-during-modal regression gate -- issue #904. +# +# tests/modal-quit-regression/run.sh --binary build/qelectrotech +# +# WHAT IT GUARDS +# +# QETDiagramEditor::openAndAddProject() shows BackupDialog as a stack object +# parented to the editor and exec()s it; QET::QetMessageBox does the same for +# every message box. exec() runs a NESTED event loop. If the editor is closed +# during that loop, WA_DeleteOnClose becomes a deleteLater() that the nested +# loop processes: ~QWidget() deletes the editor's children, the stack-allocated +# dialog among them, and the process aborts ("free(): invalid size"). +# QETMainWindow::refuseCloseWhileModal() refuses such a close. +# +# HOW IT TRIGGERS THE BUG WITHOUT A MOUSE +# +# On Linux the menu bar belongs to the window the modal blocks, and Qt ignores +# window-manager close requests for a blocked window, so there is nothing to +# click. The reported route is macOS, where File > Quit moves to the +# application menu and stays usable. What that route does is call close() +# programmatically while the dialog's loop is spinning, and that is what this +# test does, through gdb: +# +# 1. break on QDialog::exec(), i.e. the moment the first dialog is shown; +# 2. let it run for a moment, then interrupt it -- the main thread is now +# inside the dialog's nested loop, which is the only place the bug lives +# (a deleteLater() posted before exec() started is not processed by it); +# 3. call QETApp::quitQET(), which closes every editor; +# 4. let it run again: the unfixed build aborts within a second, the fixed +# one keeps running until the test interrupts it. +# +# Nothing here matches a window title or a window id. A title is a locale: +# tests/ipc-regression once shipped a pass that could not fail because the +# dialog it filtered by name was translated differently in Docker. +# +# It runs on the offscreen platform, so no X server or window manager is +# needed -- only gdb with Python. +# +# RESULT +# exit 0 PASS quitQET() ran inside the modal loop and the process survived +# exit 1 FAIL the process died of a signal after quitQET() +# exit 2 ERROR the scenario never happened (no dialog, gdb problem, ...) +# +set -uo pipefail + +BINARY="" +PROJECT="" +SETTLE=2 # seconds inside the dialog loop before interrupting +OBSERVE=5 # seconds to wait for a crash after quitQET() + +while [ $# -gt 0 ]; do + case "$1" in + --binary) BINARY="$2"; shift 2 ;; + --project) PROJECT="$2"; shift 2 ;; + *) echo "unknown argument: $1" >&2; exit 2 ;; + esac +done + +[ -n "$BINARY" ] || { echo "usage: $0 --binary [--project ]" >&2; exit 2; } +[ -x "$BINARY" ] || { echo "not executable: $BINARY" >&2; exit 2; } +BINARY="$(readlink -f "$BINARY")" + +if [ -z "$PROJECT" ]; then + SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" + PROJECT="$(ls "$SCRIPT_DIR"/../../examples/*.qet 2>/dev/null | head -1)" +fi +[ -f "$PROJECT" ] || { echo "no project found; pass --project" >&2; exit 2; } + +command -v gdb >/dev/null || { echo "missing required tool: gdb" >&2; exit 2; } +gdb -batch -ex 'python import gdb' >/dev/null 2>&1 \ + || { echo "gdb has no Python support" >&2; exit 2; } + +SANDBOX="$(mktemp -d /tmp/qet-modal-quit.XXXXXX)" +cleanup() { [ "${KEEP_LOGS:-0}" = "1" ] || rm -rf "$SANDBOX"; } +trap cleanup EXIT + +# A unique binary path gives this run its own SingleApplication socket, so it +# can neither be captured by nor capture a QElectroTech already running. A +# symlink would not do: applicationFilePath() resolves it back to the real path. +TEST_BINARY="$SANDBOX/qelectrotech-modalquit" +cp "$BINARY" "$TEST_BINARY" || { echo "could not copy binary" >&2; exit 2; } + +cp "$PROJECT" "$SANDBOX/project.qet" || { echo "could not copy project" >&2; exit 2; } + +export HOME="$SANDBOX/home" +export XDG_CONFIG_HOME="$HOME/.config" +export XDG_DATA_HOME="$HOME/.local/share" +mkdir -p "$XDG_CONFIG_HOME" "$XDG_DATA_HOME" +export QT_QPA_PLATFORM=offscreen + +cat > "$SANDBOX/scenario.gdb" < "$SANDBOX/gdb.log" 2>&1 & +GDB_PID=$! +( sleep 120; kill -9 "$GDB_PID" 2>/dev/null ) & +WATCHDOG_PID=$! +wait "$GDB_PID" +kill "$WATCHDOG_PID" 2>/dev/null + +log="$SANDBOX/gdb.log" +if ! grep -q "QET_TEST: dialog exec() entered" "$log"; then + echo "ERROR: no dialog was ever shown -- the scenario did not happen" + KEEP_LOGS=1; echo "log kept: $log"; exit 2 +fi +if ! grep -q "QET_TEST: quitQET() returned" "$log"; then + echo "ERROR: quitQET() was not called inside the dialog loop" + KEEP_LOGS=1; echo "log kept: $log"; exit 2 +fi + +sig="$(sed -n 's/^QET_TEST: final signal \([0-9]*\)$/\1/p' "$log" | tail -1)" +case "$sig" in + 2) + echo "PASS: closing the editor during a modal dialog was refused; the process survived" + exit 0 ;; + "") + echo "ERROR: could not tell how the run ended" + KEEP_LOGS=1; echo "log kept: $log"; exit 2 ;; + *) + echo "FAIL: the process died of signal $sig after quitQET() ran inside the dialog loop (issue #904)" + grep -m1 -E "free\(\)|double free|corrupted" "$log" | sed 's/^/ /' + sed -n '/^QET_TEST: final signal/,$p' "$log" | grep -E '^#[0-9]+ ' | sed 's/^/ /' + KEEP_LOGS=1; echo "log kept: $log"; exit 1 ;; +esac