From 31730678c50003d3923ed84fc0a95379893d3a91 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Mon, 19 Oct 2026 18:27:38 +1300 Subject: [PATCH 1/2] Fix a crash on clicking a sheet of a project closed without saving Closing a modified project and choosing Discard left the project in the project tree. The panel removed its item, but the project still emits projectInformationsChanged() while it is torn down (setModified()), and GenericPanel answered by adding the project back, with items for its sheets. Once the project was freed, a click on one of those sheets called diagram->showMe() on a freed Diagram: SIGSEGV in QObjectPrivate::maybeSignalConnected(), the backtrace reported on the forum (topic 3215). The panel now stops listening to a project when it is closed, to its sheets and to its embedded title block collection, before removing its item. Co-Authored-By: Claude Opus 5.5 --- sources/elementspanel.cpp | 16 ++++- tests/qttest/CMakeLists.txt | 4 ++ .../qttest/tst_elementspanelclosedproject.cpp | 72 +++++++++++++++++++ 3 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 tests/qttest/tst_elementspanelclosedproject.cpp diff --git a/sources/elementspanel.cpp b/sources/elementspanel.cpp index 349f1e9b6..2256de362 100644 --- a/sources/elementspanel.cpp +++ b/sources/elementspanel.cpp @@ -422,10 +422,20 @@ void ElementsPanel::projectWasOpened(QETProject *project) { @param project Projet a enlever du panel */ void ElementsPanel::projectWasClosed(QETProject *project) { - if (QTreeWidgetItem *item_to_remove = itemForProject(project)) { - GenericPanel::deleteItem(item_to_remove); - projects_to_display_.remove(project); + //Stop listening to the project and its sheets: the project still + //emits while it is torn down (setModified() -> + //projectInformationsChanged()), which put its item back, with items + //for sheets about to be freed + if (project) { + disconnect(project, nullptr, this, nullptr); + disconnect(project->embeddedTitleBlockTemplatesCollection(), nullptr, this, nullptr); } + if (QTreeWidgetItem *item_to_remove = itemForProject(project)) { + for (QTreeWidgetItem *sheet : childItems(item_to_remove, QET::Diagram)) + disconnect(valueForItem(sheet), nullptr, this, nullptr); + GenericPanel::deleteItem(item_to_remove); + } + projects_to_display_.remove(project); emit(panelContentChanged()); } diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index 25dedbbbe..c2566723f 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -1445,3 +1445,7 @@ if(QET_HAS_SCRIPTING) "QET_TEST_BINARY_PATH=\"$\"" "QET_EXAMPLES_DIR=\"${QET_DIR}/examples\"") endif() + +# Closing a modified project with Discard left it in the project tree, with +# items for its freed sheets; a click on one crashed (forum topic 3215). +qet_add_app_test(tst_elementspanelclosedproject tst_elementspanelclosedproject.cpp) diff --git a/tests/qttest/tst_elementspanelclosedproject.cpp b/tests/qttest/tst_elementspanelclosedproject.cpp new file mode 100644 index 000000000..b31a1697a --- /dev/null +++ b/tests/qttest/tst_elementspanelclosedproject.cpp @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include "qetapptest.h" + +#include "elementspanel.h" +#include "elementspanelwidget.h" +#include "genericpanel.h" +#include "projectview.h" +#include "qetproject.h" + +#include +#include +#include +#include + +// Closing a modified project and choosing Discard left the project in the +// project tree: the project still emitted projectInformationsChanged() while +// it was torn down, and the panel added it back, with items for its sheets. +// A click on one of those sheets then crashed in diagram->showMe() on the +// freed sheet (forum topic 3215, issue #1548). +class tst_elementspanelclosedproject : public QObject +{ + Q_OBJECT + + private slots: + void discardedProjectLeavesTheTree() + { + QETDiagramEditor *editor = QetAppTest::editor(); + QVERIFY(editor); + auto *panel_widget = editor->findChild(); + QVERIFY(panel_widget); + ElementsPanel &panel = panel_widget->elementsPanel(); + // What the app does 250 ms after the panel is first shown. + panel.reload(); + + QVERIFY(editor->newProject()); + QPointer project = editor->currentProjectView()->project(); + project->addNewDiagram(); + QVERIFY(project->projectWasModified()); + QTreeWidgetItem *project_item = panel.itemForProject(project); + QVERIFY(project_item); + + // Answer "Discard" to "Do you want to save changes?". + bool answered = false; + QTimer answer; + connect(&answer, &QTimer::timeout, this, [&answered]() { + if (auto *box = qobject_cast(QApplication::activeModalWidget())) { + if (QAbstractButton *discard = box->button(QMessageBox::Discard)) { + answered = true; + discard->click(); + } + } + }); + answer.start(20); + + // Close it from the project tree's context menu. + panel.setCurrentItem(project_item); + panel_widget->closeProject(); + answer.stop(); + QVERIFY(answered); + QTRY_VERIFY(project.isNull()); + + for (int i = 0; i < panel.topLevelItemCount(); ++i) { + QTreeWidgetItem *item = panel.topLevelItem(i); + QVERIFY2(item->type() != QET::Project, + qPrintable(QStringLiteral("the closed project is still in the tree: %1").arg(item->text(0)))); + } + } +}; + +QET_APP_TEST_MAIN(tst_elementspanelclosedproject) + +#include "tst_elementspanelclosedproject.moc" From 46d8588113ad2aee4c868c82d2056e686b6e7be0 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Mon, 19 Oct 2026 18:27:38 +1300 Subject: [PATCH 2/2] Fix a closed project staying in the element collection panel A project opened before the element collection panel's first load is queued until the load starts; one opened during a load goes into the model being loaded. ElementsCollectionWidget::removeProject() only removed the project from the model already shown, so a project closed in either window stayed in the collection, and the next reload read the freed project (crash in XmlElementCollection::root()). removeProject() now also takes the project out of the queue and out of the model being loaded. That model waits for its loading threads, which read the project, and drops the project's items from the list applied when the loading ends. Co-Authored-By: Claude Opus 5.5 --- .../elementscollectionmodel.cpp | 7 ++ .../elementscollectionwidget.cpp | 5 ++ tests/qttest/CMakeLists.txt | 4 + tests/qttest/tst_collectionclosedproject.cpp | 81 +++++++++++++++++++ 4 files changed, 97 insertions(+) create mode 100644 tests/qttest/tst_collectionclosedproject.cpp diff --git a/sources/ElementsCollection/elementscollectionmodel.cpp b/sources/ElementsCollection/elementscollectionmodel.cpp index 440f10417..d2e01da17 100644 --- a/sources/ElementsCollection/elementscollectionmodel.cpp +++ b/sources/ElementsCollection/elementscollectionmodel.cpp @@ -526,6 +526,13 @@ void ElementsCollectionModel::removeProject(QETProject *project) if (!m_project_list.contains(project)) return; + //A loading may be setting up this project's items on worker + //threads: let it end, and keep the items about to be deleted out of + //the list applied when it is finished. + m_future.waitForFinished(); + for (ElementCollectionItem *eci : projectItems(project)) + m_items_list_to_setUp.removeOne(eci); + int row = m_project_list.indexOf(project); if (removeRows(row, 1, QModelIndex())) { m_project_list.removeOne(project); diff --git a/sources/ElementsCollection/elementscollectionwidget.cpp b/sources/ElementsCollection/elementscollectionwidget.cpp index 98d6b83c9..7d029d593 100644 --- a/sources/ElementsCollection/elementscollectionwidget.cpp +++ b/sources/ElementsCollection/elementscollectionwidget.cpp @@ -141,6 +141,11 @@ void ElementsCollectionWidget::addProject(QETProject *project) } void ElementsCollectionWidget::removeProject(QETProject *project) { + //Before the first load and during a load, the project is also + //waiting to be loaded, or in the model being loaded + m_waiting_project.removeAll(project); + if (m_new_model) + m_new_model->removeProject(project); if (m_model) m_model->removeProject(project); } diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index c2566723f..55b60d702 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -1449,3 +1449,7 @@ endif() # Closing a modified project with Discard left it in the project tree, with # items for its freed sheets; a click on one crashed (forum topic 3215). qet_add_app_test(tst_elementspanelclosedproject tst_elementspanelclosedproject.cpp) + +# A project closed before the element collection panel's first load, or during +# a load, stayed in the collection and the next reload read the freed project. +qet_add_app_test(tst_collectionclosedproject tst_collectionclosedproject.cpp) diff --git a/tests/qttest/tst_collectionclosedproject.cpp b/tests/qttest/tst_collectionclosedproject.cpp new file mode 100644 index 000000000..d4c5d54f7 --- /dev/null +++ b/tests/qttest/tst_collectionclosedproject.cpp @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include "qetapptest.h" + +#include "ElementsCollection/elementscollectionmodel.h" +#include "ElementsCollection/elementscollectionwidget.h" +#include "qetproject.h" + +#include + +// A project closed while the element collection panel has not loaded yet, or +// while it is loading, stayed in the collection: the next reload read the +// freed project (crash in XmlElementCollection::root() from +// ElementsCollectionWidget::reload()). +class tst_collectionclosedproject : public QObject +{ + Q_OBJECT + + /// The collection's tree (not the macros one): reload() disables it + /// until the loading has finished. + static QTreeView *loadingView(ElementsCollectionWidget &w) + { + for (QTreeView *v : w.findChildren()) + if (!v->isEnabled()) return v; + return nullptr; + } + + /// Reload and wait for the loading to finish. + static ElementsCollectionModel *reloadAndWait(ElementsCollectionWidget &w) + { + w.reload(); + QTreeView *main_view = loadingView(w); + if (!main_view) return nullptr; + if (!QTest::qWaitFor([main_view]() { return main_view->isEnabled(); }, 300000)) + return nullptr; + return qobject_cast(main_view->model()); + } + + private slots: + void closedBeforeFirstLoad() + { + ElementsCollectionWidget w; + QETProject kept; + QETProject closed; + w.addProject(&kept); + w.addProject(&closed); + w.removeProject(&closed); + + ElementsCollectionModel *model = reloadAndWait(w); + QVERIFY(model); + QVERIFY(model->project().contains(&kept)); + QVERIFY2(!model->project().contains(&closed), + "a project closed before the first load is still in the collection"); + } + + void closedWhileLoading() + { + ElementsCollectionWidget w; + QETProject kept; + QETProject closed; + QVERIFY(reloadAndWait(w)); + w.addProject(&kept); + w.addProject(&closed); + + // A second load starts; the project is closed before it ends. + w.reload(); + QTreeView *main_view = loadingView(w); + QVERIFY(main_view); + w.removeProject(&closed); + QTRY_VERIFY_WITH_TIMEOUT(main_view->isEnabled(), 300000); + + auto *model = qobject_cast(main_view->model()); + QVERIFY(model); + QVERIFY(model->project().contains(&kept)); + QVERIFY2(!model->project().contains(&closed), + "a project closed during a load is still in the collection"); + } +}; + +QET_APP_TEST_MAIN(tst_collectionclosedproject) + +#include "tst_collectionclosedproject.moc"