Repository navigation
Remove MainWindow's own event filters before its members are destroyed - #7047
whosawhatsis wants to merge 2 commits into
Conversation
MainWindow stays installed as an event filter while its own members are being destroyed, so MainWindow::eventFilter() can run against a destroyed member and read freed memory. It is silent in an ordinary build and a heap-use-after-free under AddressSanitizer. The test opens a second window, closes it, and checks the window count returns to what it was. It passes in an ordinary build and fails under AddressSanitizer until the event filter is removed before the members are torn down. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MainWindow installs itself as an event filter on itself and on the find panel. Destroying a member such as rubberBandManager reparents the widget it owns, and reparenting delivers events, which re-enter MainWindow::eventFilter() while that member is being destroyed. eventFilter() calls rubberBandManager.isVisible(), so it reads freed memory: a heap-use-after-free under AddressSanitizer, and a silent read of stale memory in an ordinary build. The destructor already severs the dock connections and filters for exactly this reason; do the same for the two filters we install on ourselves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
I don't think this is the correct solution for this case. The |
|
Not with Qt's public API, unfortunately. There is no Qt does clean this up automatically — but in
If you would rather it not depend on that set staying complete, the small version is to route the class EventFilterGuard {
public:
explicit EventFilterGuard(QObject *filter) : filter_(filter) {}
void install(QObject *target) { target->installEventFilter(filter_); targets_.append(target); }
~EventFilterGuard() { for (auto& t : targets_) if (t) t->removeEventFilter(filter_); }
private:
QObject *filter_;
QList<QPointer<QObject>> targets_;
};That makes a future |
User documentation for the wiki is in the last section.
Summary
Closing a window reads freed memory.
MainWindowinstalls itself as an event filter on itself (MainWindow.cc,installEventFilter(this))and on the find panel, and neither filter is removed when the window is destroyed. Destroying a member
such as
rubberBandManagerreparents the widget it owns; reparenting delivers events; those eventsreach
MainWindow::eventFilter(), which callsrubberBandManager.isVisible()— on the member that isat that moment being destroyed.
Under AddressSanitizer this is a
heap-use-after-free. In an ordinary build the freed block is usuallystill intact, so the read silently returns stale data, which is why it has gone unnoticed.
The fix
~MainWindowalready severs the dock connections and dock event filters for exactly this reason, witha comment describing the hazard. That fix simply missed the two filters the window installs on itself:
Guarding inside
eventFilter()instead would leave the re-entrancy in place and require a"being destroyed" flag; removing the filters ends it at the source and matches the existing code.
How this was found
Running the GUI test suite under AddressSanitizer while investigating an unrelated test-infrastructure
issue. It is independent of that work: it reproduces on master as soon as a test closes a window.
Testing
TestMainWindow::checkClosingWindowDoesNotUseFreedMembers(): opens a second window, closes it,and checks the window count returns to its previous value. It passes in an ordinary build — the
failure it exists to catch is the sanitizer report — so it was confirmed failing first: before the
fix ASan reported the use-after-free with both the allocation and the free traced into this test's
own lines; after the fix,
--run-all-gui-testsis ASan-clean with 0 failures.-DENABLE_GUI_TESTS=1 -DCMAKE_BUILD_TYPE=RelWithDebInfo,-fsanitize=address -fno-omit-frame-pointer -g, run withASAN_OPTIONS=detect_stack_use_after_return=1:detect_leaks=0.ctest -j8on macOS: 2758/2761, the only failures being the 3 known SVG arc tessellation tests thatalso fail on master.
./scripts/beautify.sh --checkclean with clang-format 18.1.8.User documentation (for the openscad.org wiki)
Closing a window no longer reads memory belonging to parts of that window that have already been torn
down. The effect was not reliably visible — usually nothing appeared to go wrong, since the freed
memory was typically still intact — but it could produce a crash on window close. No setting or
workflow changes.
🤖 Generated with Claude Code