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/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..55b60d702 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -1445,3 +1445,11 @@ 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) + +# 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" 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"