Fix macos crash (trikset#1746)

Fixed trikset#1707 Problem description: dragging an element using QDrag->exec() crashed the program, because by the time DraggableElement was called, it automatically managed to free itself. Even though the palette has been updated with Qt::QueuedConnection, which made it possible to call mousePressEvent using drag->exec before cleaning the elements, the drag->exec call is not blocked in macOS. As a solution, it was possible to block the event loop for drag->exec, but instead it was decided to stupidly emit to update the palette after the corresponding operations (since cleaning is put after mousePressEvent in the event loop). P.S. No problems with memory leaks of mPaletteElements and mPaletteItems were found, they are cleaned when QTreeWidget::clear() is called. However, they have problems with hashing.
iakov · Jul 22, 2024 · 06e1c23 · 06e1c23
1 parent b79bea4
commit 06e1c23
Show file tree

Hide file tree

Showing 8 changed files with 55 additions and 7 deletions.
diff --git a/qrgui/mainWindow/palette/draggableElement.cpp b/qrgui/mainWindow/palette/draggableElement.cpp
@@ -56,6 +56,7 @@ DraggableElement::DraggableElement(MainWindow &mainWindow
 	, mEditorManagerProxy(editorManagerProxy)
 	, mMainWindow(mainWindow)
 {
+	readyForDelete = true;
 	QHBoxLayout *layout = new QHBoxLayout(this);
 	layout->setContentsMargins(0, 4, 0, 4);
 
@@ -97,6 +98,11 @@ DraggableElement::DraggableElement(MainWindow &mainWindow
 	setObjectName(mData.name());
 }
 
+bool DraggableElement::getReadyForDelete() const
+{
+	return readyForDelete;
+}
+
 QIcon DraggableElement::icon() const
 {
 	return mData.icon();
@@ -375,6 +381,7 @@ void DraggableElement::mousePressEvent(QMouseEvent *event)
 			menu->exec(QCursor::pos());
 		}
 	} else {
+		readyForDelete = false;
 		QDrag *drag = new QDrag(this);
 		drag->setMimeData(mimeData(elementId));
 
@@ -385,6 +392,8 @@ void DraggableElement::mousePressEvent(QMouseEvent *event)
 		}
 
 		drag->exec(Qt::CopyAction);
+		readyForDelete = true;
+		emit signalReadyForDelete();
 	}
 }
 

diff --git a/qrgui/mainWindow/palette/draggableElement.h b/qrgui/mainWindow/palette/draggableElement.h
@@ -66,6 +66,11 @@ class DraggableElement : public QWidget
 	/// Returns a mime data instance binded with object during drag-and-drop.
 	QMimeData *mimeData(const Id &elementId) const;
 
+	bool getReadyForDelete() const;
+
+signals:
+	void signalReadyForDelete();
+
 private slots:
 	void changePropertiesPaletteActionTriggered();
 	void changeDynamicPropertiesPaletteActionTriggered();
@@ -104,6 +109,7 @@ private slots:
 	MainWindow &mMainWindow;
 	Id mDeletedElementId;
 	bool mIsRootDiagramNode {};
+	bool readyForDelete;
 };
 
 }

diff --git a/qrgui/mainWindow/palette/paletteTreeWidget.cpp b/qrgui/mainWindow/palette/paletteTreeWidget.cpp
@@ -35,6 +35,18 @@ PaletteTreeWidget::PaletteTreeWidget(PaletteTree &palette, MainWindow &mainWindo
 	mEditorManager = &editorManagerProxy;
 }
 
+bool PaletteTreeWidget::readyToRefresh()
+{
+	for (auto& draggableElement: mDraggableElements) {
+		if (!draggableElement->getReadyForDelete()) {
+			connect(draggableElement, &DraggableElement::signalReadyForDelete,
+				this, &PaletteTreeWidget::signalReadyToRefresh);
+			return false;
+		}
+	}
+	return true;
+}
+
 void PaletteTreeWidget::addGroups(QList<QPair<QString, QList<PaletteElement>>> &groups
 		, QMap<QString, QString> const &descriptions
 		, bool hideIfEmpty
@@ -45,6 +57,7 @@ void PaletteTreeWidget::addGroups(QList<QPair<QString, QList<PaletteElement>>> &
 	mPaletteElements.clear();
 	mElementsSet.clear();
 	mItemsVisible.clear();
+	mDraggableElements.clear();
 
 	if (groups.isEmpty() && hideIfEmpty) {
 		hide();
@@ -96,6 +109,7 @@ void PaletteTreeWidget::addItemType(const PaletteElement &data, QTreeWidgetItem
 	mElementsSet.insert(data);
 	mPaletteElements.insert(data.id(), element);
 	mPaletteItems.insert(data.id(), leaf);
+	mDraggableElements.append(element);
 
 	parent->addChild(leaf);
 	setItemWidget(leaf, 0, element);

diff --git a/qrgui/mainWindow/palette/paletteTreeWidget.h b/qrgui/mainWindow/palette/paletteTreeWidget.h
@@ -74,6 +74,11 @@ class PaletteTreeWidget : public QTreeWidget
 	/// Returns set that may let quickly determine should we update palette or no.
 	const QSet<PaletteElement> &elementsSet() const;
 
+	bool readyToRefresh();
+
+signals:
+	void signalReadyToRefresh();
+
 protected:
 	void mousePressEvent(QMouseEvent *event);
 
@@ -122,6 +127,7 @@ class PaletteTreeWidget : public QTreeWidget
 	QHash<Id, DraggableElement *> mPaletteElements;  // Takes ownership.
 	QHash<Id, QTreeWidgetItem *> mPaletteItems;  // Takes ownership.
 	QHash<QTreeWidgetItem *, bool> mItemsVisible;
+	QList<DraggableElement *> mDraggableElements; // Takes ownership
 };
 
 }

diff --git a/qrgui/mainWindow/palette/paletteTreeWidgets.cpp b/qrgui/mainWindow/palette/paletteTreeWidgets.cpp
@@ -112,7 +112,7 @@ void PaletteTreeWidgets::initUserTree()
 {
 	refreshUserPalette();
 	connect(&mMainWindow->models().exploser(), &models::Exploser::explosionsSetCouldChange
-			, this, [&](){refreshUserPalette(true);});
+			, this, [&](){refreshUserPaletteHandler(true);});
 }
 
 void PaletteTreeWidgets::addTopItemType(const PaletteElement &data, QTreeWidget *tree)
@@ -241,6 +241,17 @@ void PaletteTreeWidgets::customizeExplosionTitles(const QString &userGroupTitle,
 	mUserGroupDescription = userGroupDescription;
 }
 
+void PaletteTreeWidgets::refreshUserPaletteHandler(bool force)
+{
+	if (mUserTree->readyToRefresh()) {
+		refreshUserPalette(force);
+		return;
+	}
+	connect(mUserTree, &PaletteTreeWidget::signalReadyToRefresh, this, [=]() {
+		refreshUserPalette(force);
+	});
+}
+
 void PaletteTreeWidgets::refreshUserPalette(bool force)
 {
 	QList<QPair<QString, QList<gui::PaletteElement>>> groups;

diff --git a/qrgui/mainWindow/palette/paletteTreeWidgets.h b/qrgui/mainWindow/palette/paletteTreeWidgets.h
@@ -68,6 +68,8 @@ class PaletteTreeWidgets : public QSplitter
 	/// Rereads user blocks information.
 	void refreshUserPalette(bool force = false);
 
+	void refreshUserPaletteHandler(bool force = false);
+
 	/// Sets user palette header and description.
 	void customizeExplosionTitles(const QString &userGroupTitle
 			, const QString &userGroupDescription);

diff --git a/qrtranslations/fr/qrgui_mainWindow_fr.ts b/qrtranslations/fr/qrgui_mainWindow_fr.ts
@@ -1006,12 +1006,12 @@ WARNING: The settings will be restored after application restart</source>
 <context>
     <name>qReal::gui::DraggableElement</name>
     <message>
-        <location filename="../../qrgui/mainWindow/palette/draggableElement.cpp" line="+86"/>
+        <location filename="../../qrgui/mainWindow/palette/draggableElement.cpp" line="+87"/>
         <source>Mouse gesture</source>
         <translation>Geste souris</translation>
     </message>
     <message>
-        <location line="+88"/>
+        <location line="+93"/>
         <source>Deleting an element: </source>
         <translation>Supression d&apos;un élément :</translation>
     </message>
@@ -1121,7 +1121,7 @@ WARNING: The settings will be restored after application restart</source>
         <translation type="obsolete">Ajouter un élément</translation>
     </message>
     <message>
-        <location filename="../../qrgui/mainWindow/palette/paletteTreeWidget.cpp" line="+143"/>
+        <location filename="../../qrgui/mainWindow/palette/paletteTreeWidget.cpp" line="+157"/>
         <source>Add Entity</source>
         <translation type="unfinished"></translation>
     </message>

diff --git a/qrtranslations/ru/qrgui_mainWindow_ru.ts b/qrtranslations/ru/qrgui_mainWindow_ru.ts
@@ -1120,12 +1120,12 @@ WARNING: The settings will be restored after application restart</source>
 <context>
     <name>qReal::gui::DraggableElement</name>
     <message>
-        <location filename="../../qrgui/mainWindow/palette/draggableElement.cpp" line="+86"/>
+        <location filename="../../qrgui/mainWindow/palette/draggableElement.cpp" line="+87"/>
         <source>Mouse gesture</source>
         <translation>Жест мышью</translation>
     </message>
     <message>
-        <location line="+88"/>
+        <location line="+93"/>
         <source>Deleting an element: </source>
         <translation>Удаление элемента: </translation>
     </message>
@@ -1250,7 +1250,7 @@ WARNING: The settings will be restored after application restart</source>
         <translation type="obsolete">Добавить элемент</translation>
     </message>
     <message>
-        <location filename="../../qrgui/mainWindow/palette/paletteTreeWidget.cpp" line="+143"/>
+        <location filename="../../qrgui/mainWindow/palette/paletteTreeWidget.cpp" line="+157"/>
         <source>Add Entity</source>
         <translation>Добавить сущность</translation>
     </message>