From aac28b280f1605657189dd3885cab17e2908b336 Mon Sep 17 00:00:00 2001 From: Axel Spoerl Date: Mon, 7 Apr 2025 13:58:43 +0200 Subject: [PATCH] QDockWidget: Fix memory leak when closing floating tabs QDockWidgetGroupWindow (aka "floating tabs") can have a single QDockWidget child only, when a second QDockWidget is hovering over it. When the QDockWidgetGroupWindow detects the disappearance of its second last child, it reparents the last child to the QMainWindow and makes it floating. It removes its last dock widget child from its own item_list and itself from its parent's (the main window's) item_list. Then the QDockWidgetGroupWindow removes itself by calling its deleteLater() slot. A removal from an item_list calls the d'tor of QDockAreaLayoutItem, which deletes the item's subinfo and placeholder item, if they exist. It does not delete the item's widgetItem. In fact the layout accesses the widgetItem member to draw placeholders and decorations. As a consequence, both the QDockWidgetGroupWindowItem and the QDockWidgetItem are leaked, when the corresponding record is removed. Implement QDockAreaLayout::takeWidgetItem(), which transfers ownership of QDockAreaLayoutItem::widgetItem to the caller by returning a std::unique_ptr. Call this method in QDockWidgetGroupWindow::destroyOrHideIfEmpty() and QDockWidgetGroupWindow::reparentToMainWindow(). As a drive-by, use static_cast() and assert a qobject_cast in debug mode. Fixes: QTBUG-135442 Pick-to: 6.5 Change-Id: I055c998f515f2bb461518b7516f56db4673687da Reviewed-by: Richard Moe Gustavsen (cherry picked from commit 40766380224b3a38bbf92b72d8d9baa3762bc06a) Reviewed-by: Qt Cherry-pick Bot (cherry picked from commit 5cafb9c760e6381b75e70d5c5f8328e5e62bef5e) --- src/widgets/widgets/qdockarealayout.cpp | 8 ++++++++ src/widgets/widgets/qdockarealayout_p.h | 1 + src/widgets/widgets/qmainwindowlayout.cpp | 19 +++++++++++-------- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/widgets/widgets/qdockarealayout.cpp b/src/widgets/widgets/qdockarealayout.cpp index 32d0f9b345a..961530dec9d 100644 --- a/src/widgets/widgets/qdockarealayout.cpp +++ b/src/widgets/widgets/qdockarealayout.cpp @@ -1501,6 +1501,14 @@ QList QDockAreaLayoutInfo::indexOf(QWidget *widget) const return QList(); } +std::unique_ptr QDockAreaLayoutInfo::takeWidgetItem(QWidget *widget) +{ + std::unique_ptr widgetItem; + if (const auto path = indexOf(widget); !path.isEmpty()) + widgetItem.reset(item(path).widgetItem); + return widgetItem; +} + QMainWindowLayout *QDockAreaLayoutInfo::mainWindowLayout() const { QMainWindowLayout *result = qt_mainwindow_layout(mainWindow); diff --git a/src/widgets/widgets/qdockarealayout_p.h b/src/widgets/widgets/qdockarealayout_p.h index aa39bf9913c..7ebe3a294f5 100644 --- a/src/widgets/widgets/qdockarealayout_p.h +++ b/src/widgets/widgets/qdockarealayout_p.h @@ -145,6 +145,7 @@ public: QList indexOf(QWidget *widget) const; QList indexOfPlaceHolder(const QString &objectName) const; + std::unique_ptr takeWidgetItem(QWidget *widget); QDockWidget *apply(bool animate); diff --git a/src/widgets/widgets/qmainwindowlayout.cpp b/src/widgets/widgets/qmainwindowlayout.cpp index 22ee3dbc5e4..627a9a3abdf 100644 --- a/src/widgets/widgets/qmainwindowlayout.cpp +++ b/src/widgets/widgets/qmainwindowlayout.cpp @@ -471,6 +471,12 @@ void QDockWidgetGroupWindow::destroyOrHideIfEmpty() if (!wasHidden) dw->show(); } + Q_ASSERT(qobject_cast(parentWidget())); + auto *mainWindow = static_cast(parentWidget()); + QMainWindowLayout *mwLayout = qt_mainwindow_layout(mainWindow); + QDockAreaLayoutInfo &parentInfo = mwLayout->layoutState.dockAreaLayout.docks[layoutInfo()->dockPos]; + std::unique_ptr cleanup = parentInfo.takeWidgetItem(this); + parentInfo.remove(this); deleteLater(); } @@ -714,7 +720,8 @@ void QDockWidgetGroupWindow::destroyIfSingleItemLeft() if (layoutInfo()->indexOf(lastDockWidget).isEmpty()) return; - auto *mainWindow = qobject_cast(parentWidget()); + Q_ASSERT(qobject_cast(parentWidget())); + auto *mainWindow = static_cast(parentWidget()); QMainWindowLayout *mwLayout = qt_mainwindow_layout(mainWindow); // Unplug the last remaining dock widget and hide the group window, to avoid flickering @@ -722,17 +729,12 @@ void QDockWidgetGroupWindow::destroyIfSingleItemLeft() lastDockWidget->setGeometry(geometry()); hide(); - // Get the layout info for the main window dock, where dock widgets need to go - QDockAreaLayoutInfo &parentInfo = mwLayout->layoutState.dockAreaLayout.docks[layoutInfo()->dockPos]; - // Re-parent last dock widget reparentToMainWindow(lastDockWidget); // the group window could still have placeholder items => clear everything layoutInfo()->item_list.clear(); - // remove the group window and the dock's item_list pointing to it. - parentInfo.remove(this); destroyOrHideIfEmpty(); } @@ -742,13 +744,14 @@ void QDockWidgetGroupWindow::reparentToMainWindow(QDockWidget *dockWidget) // - remove it from the floating dock's layout info // - insert it to the main dock's layout info // Finally, set draggingDock to nullptr, since the drag is finished. - auto *mainWindow = qobject_cast(parentWidget()); - Q_ASSERT(mainWindow); + Q_ASSERT(qobject_cast(parentWidget())); + auto *mainWindow = static_cast(parentWidget()); QMainWindowLayout *mwLayout = qt_mainwindow_layout(mainWindow); Q_ASSERT(mwLayout); QDockAreaLayoutInfo &parentInfo = mwLayout->layoutState.dockAreaLayout.docks[layoutInfo()->dockPos]; dockWidget->removeEventFilter(this); parentInfo.add(dockWidget); + std::unique_ptr cleanup = layoutInfo()->takeWidgetItem(dockWidget); layoutInfo()->remove(dockWidget); const bool wasFloating = dockWidget->isFloating(); const bool wasVisible = dockWidget->isVisible();