Merge pull request #1549 from ispyisail/fix/closed-project-panels

Fix a crash after closing a project without saving (forum 3215)

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

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);
 }

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<Diagram *>(sheet), nullptr, this, nullptr);
+		GenericPanel::deleteItem(item_to_remove);
+	}
+	projects_to_display_.remove(project);
 	emit(panelContentChanged());
 }
 

tests/qttest/CMakeLists.txt

@@ -1445,3 +1445,11 @@ if(QET_HAS_SCRIPTING)
     "QET_TEST_BINARY_PATH=\"$<TARGET_FILE:qelectrotech>\""
     "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)

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 <QTreeView>
+
+// 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<QTreeView *>())
+			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<ElementsCollectionModel *>(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<ElementsCollectionModel *>(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"

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 <QAbstractButton>
+#include <QMessageBox>
+#include <QPointer>
+#include <QTimer>
+
+// 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<ElementsPanelWidget *>();
+			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<QETProject> 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<QMessageBox *>(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"