From 5cafb9c760e6381b75e70d5c5f8328e5e62bef5e 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.8 6.5 Change-Id: I055c998f515f2bb461518b7516f56db4673687da Reviewed-by: Richard Moe Gustavsen (cherry picked from commit 40766380224b3a38bbf92b72d8d9baa3762bc06a) Reviewed-by: Qt Cherry-pick Bot --- 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 e0d8995bf83..d5ed9ad368f 100644 --- a/src/widgets/widgets/qdockarealayout.cpp +++ b/src/widgets/widgets/qdockarealayout.cpp @@ -1501,6 +1501,14 @@ QList QDockAreaLayoutInfo::indexOf(const 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 ca9f4b656f0..587d7077a5f 100644 --- a/src/widgets/widgets/qdockarealayout_p.h +++ b/src/widgets/widgets/qdockarealayout_p.h @@ -145,6 +145,7 @@ public: QList indexOf(const 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 14a8bbd857d..b81b17a64ba 100644 --- a/src/widgets/widgets/qmainwindowlayout.cpp +++ b/src/widgets/widgets/qmainwindowlayout.cpp @@ -470,6 +470,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(); } @@ -713,7 +719,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 @@ -721,17 +728,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(); } @@ -741,13 +743,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();