From 259c3cb4d48cdfb11a17b51ae1df597f37841aac Mon Sep 17 00:00:00 2001 From: hjk Date: Thu, 24 Mar 2016 13:59:05 +0100 Subject: [PATCH] Debugger: Fix object leakage on shutdown and heap-use-after-free. Task-number: QTCREATORBUG-15938 Change-Id: I437756705c33730398a129651fabe34c92334656 Reviewed-by: Eike Ziller --- src/plugins/debugger/debuggermainwindow.cpp | 36 ++++++++----- src/plugins/debugger/debuggermainwindow.h | 14 ++---- src/plugins/debugger/debuggerplugin.cpp | 56 +++++++++++---------- src/plugins/valgrind/callgrindtool.cpp | 1 - 4 files changed, 56 insertions(+), 51 deletions(-) diff --git a/src/plugins/debugger/debuggermainwindow.cpp b/src/plugins/debugger/debuggermainwindow.cpp index 54390d844d1..50778240bdf 100644 --- a/src/plugins/debugger/debuggermainwindow.cpp +++ b/src/plugins/debugger/debuggermainwindow.cpp @@ -81,9 +81,22 @@ DebuggerMainWindow::~DebuggerMainWindow() { // as we have to setParent(0) on dock widget that are not selected, // we keep track of all and make sure we don't leak any - foreach (const DockPtr &ptr, m_dockWidgets) { - if (ptr) - delete ptr.data(); + foreach (const Perspective &perspective, m_perspectiveForPerspectiveId) { + foreach (const Perspective::Operation &operation, perspective.operations()) { + if (operation.widget) { + // There are two possible states: Either addDockForWidget(widget) has + // been on operation.widget (e.g. when the perspective gets activated) + // for the first time, or not. In the first case we delete only the + // widget, in the second case its parent, which is the dock. + if (QWidget *parent = operation.widget->parentWidget()) { + QTC_CHECK(qobject_cast(parent)); + delete parent; + } else { + // These are from perspectives that never + delete operation.widget; + } + } + } } } @@ -119,11 +132,6 @@ QDockWidget *DebuggerMainWindow::dockWidget(const QByteArray &dockId) const return m_dockForDockId.value(dockId); } -QWidget *DebuggerMainWindow::modeWindow() -{ - return m_modeWindow; -} - void DebuggerMainWindow::resetCurrentPerspective() { loadPerspectiveHelper(m_currentPerspectiveId, false); @@ -140,7 +148,7 @@ void DebuggerMainWindow::restorePerspective(const QByteArray &perspectiveId) m_perspectiveChooser->setCurrentIndex(index); } -void DebuggerMainWindow::finalizeSetup(Core::IMode *mode, QWidget *central) +void DebuggerMainWindow::finalizeSetup() { auto viewButton = new QToolButton; viewButton->setText(tr("Views")); @@ -195,7 +203,10 @@ void DebuggerMainWindow::finalizeSetup(Core::IMode *mode, QWidget *central) m_toolbarDock = dock; addDockWidget(Qt::BottomDockWidgetArea, dock); +} +QWidget *createModeWindow(Core::IMode *mode, DebuggerMainWindow *mainWindow, QWidget *central) +{ if (!central) central = new EditorManagerPlaceHolder(mode); @@ -225,7 +236,7 @@ void DebuggerMainWindow::finalizeSetup(Core::IMode *mode, QWidget *central) // Right-side window with editor, output etc. auto mainWindowSplitter = new MiniSplitter; - mainWindowSplitter->addWidget(this); + mainWindowSplitter->addWidget(mainWindow); mainWindowSplitter->addWidget(new OutputPanePlaceHolder(mode, mainWindowSplitter)); auto outputPane = new OutputPanePlaceHolder(mode, mainWindowSplitter); outputPane->setObjectName(QLatin1String("DebuggerOutputPanePlaceHolder")); @@ -242,9 +253,9 @@ void DebuggerMainWindow::finalizeSetup(Core::IMode *mode, QWidget *central) splitter->setStretchFactor(0, 0); splitter->setStretchFactor(1, 1); splitter->setObjectName(QLatin1String("DebugModeWidget")); - setCentralWidget(centralEditorWidget); + mainWindow->setCentralWidget(centralEditorWidget); - m_modeWindow = splitter; + return splitter; } void DebuggerMainWindow::loadPerspectiveHelper(const QByteArray &perspectiveId, bool fromStoredSettings) @@ -351,7 +362,6 @@ QDockWidget *DebuggerMainWindow::registerDockWidget(const QByteArray &dockId, QW QTC_ASSERT(!widget->objectName().isEmpty(), return 0); QDockWidget *dockWidget = addDockForWidget(widget); dockWidget->setParent(0); - m_dockWidgets.append(DebuggerMainWindow::DockPtr(dockWidget)); m_dockForDockId[dockId] = dockWidget; return dockWidget; } diff --git a/src/plugins/debugger/debuggermainwindow.h b/src/plugins/debugger/debuggermainwindow.h index 3c7d221a59a..28bb312580b 100644 --- a/src/plugins/debugger/debuggermainwindow.h +++ b/src/plugins/debugger/debuggermainwindow.h @@ -61,7 +61,7 @@ public: Qt::DockWidgetArea area = Qt::BottomDockWidgetArea); QByteArray dockId; - QWidget *widget = 0; + QPointer widget; QByteArray anchorDockId; OperationType operationType; bool visibleByDefault; @@ -115,14 +115,12 @@ public: void resetCurrentPerspective(); void restorePerspective(const QByteArray &perspectiveId); - void finalizeSetup(Core::IMode *mode, QWidget *central = 0); + void finalizeSetup(); void showStatusMessage(const QString &message, int timeoutMS); QDockWidget *dockWidget(const QByteArray &dockId) const; QByteArray currentPerspective() const { return m_currentPerspectiveId; } - QWidget *modeWindow(); - private: QDockWidget *registerDockWidget(const QByteArray &dockId, QWidget *widget); void loadPerspectiveHelper(const QByteArray &perspectiveId, bool fromStoredSettings = true); @@ -136,14 +134,10 @@ private: QHash m_dockForDockId; QHash m_toolbarForPerspectiveId; QHash m_perspectiveForPerspectiveId; - - // list of dock widgets to prevent memory leak - typedef QPointer DockPtr; - QList m_dockWidgets; - - QWidget *m_modeWindow = 0; }; +QWidget *createModeWindow(Core::IMode *mode, DebuggerMainWindow *mainWindow, QWidget *central); + } // Utils #endif // DEBUGGERMAINWINDOW_H diff --git a/src/plugins/debugger/debuggerplugin.cpp b/src/plugins/debugger/debuggerplugin.cpp index 7a2acc923f5..20b093a635d 100644 --- a/src/plugins/debugger/debuggerplugin.cpp +++ b/src/plugins/debugger/debuggerplugin.cpp @@ -486,41 +486,27 @@ bool DummyEngine::hasCapability(unsigned cap) const class DebugModeContext : public IContext { public: - DebugModeContext(DebuggerMainWindow *mainWindow) : m_mainWindow(mainWindow) + DebugModeContext(QWidget *modeWindow) { setContext(Context(CC::C_EDITORMANAGER)); + setWidget(modeWindow); ICore::addContextObject(this); } - - QWidget *widget() const override { return m_mainWindow->modeWindow(); } - - DebuggerMainWindow *m_mainWindow; }; class DebugMode : public IMode { public: - DebugMode(DebuggerMainWindow *mainWindow) : m_mainWindow(mainWindow) + DebugMode() { setObjectName(QLatin1String("DebugMode")); setContext(Context(C_DEBUGMODE, CC::C_NAVIGATION_PANE)); setDisplayName(DebuggerPlugin::tr("Debug")); setIcon(Utils::Icon::modeIcon(Icons::MODE_DEBUGGER_CLASSIC, Icons::MODE_DEBUGGER_FLAT, Icons::MODE_DEBUGGER_FLAT_ACTIVE)); -// setIcon(Utils::Icon::modeIcon(Icons::MODE_ANALYZE_CLASSIC, -// Icons::MODE_ANALYZE_FLAT, Icons::MODE_ANALYZE_FLAT_ACTIVE)); setPriority(85); setId(MODE_DEBUG); } - - QWidget *widget() const override { return m_mainWindow->modeWindow(); } - - ~DebugMode() - { -// delete m_widget; - } - - DebuggerMainWindow *m_mainWindow; }; /////////////////////////////////////////////////////////////////////// @@ -924,7 +910,9 @@ public: void updateActiveLanguages(); public: - DebuggerMainWindow *m_mainWindow = 0; + QPointer m_mainWindow; + QPointer m_modeWindow; + QPointer m_mode; QHash m_descriptions; ActionContainer *m_menu = 0; @@ -1051,11 +1039,6 @@ DebuggerPluginPrivate::~DebuggerPluginPrivate() delete m_breakHandler; m_breakHandler = 0; - -// delete m_debugMode; -// m_debugMode = 0; - delete m_mainWindow; - m_mainWindow = 0; } DebuggerEngine *DebuggerPluginPrivate::dummyEngine() @@ -1748,13 +1731,16 @@ bool DebuggerPluginPrivate::initialize(const QStringList &arguments, connect(ProjectExplorerPlugin::instance(), &ProjectExplorerPlugin::settingsChanged, this, &DebuggerPluginPrivate::updateDebugWithoutDeployMenu); + m_mainWindow->finalizeSetup(); + // Debug mode setup - auto mode = new DebugMode(m_mainWindow); + m_mode = new DebugMode; + m_modeWindow = createModeWindow(m_mode, m_mainWindow, 0); + m_mode->setWidget(m_modeWindow); (void) new DebugModeContext(m_mainWindow); - m_mainWindow->finalizeSetup(mode); - m_plugin->addAutoReleasedObject(mode); + m_plugin->addObject(m_mode); connect(SessionManager::instance(), &SessionManager::startupProjectChanged, @@ -3047,6 +3033,23 @@ void DebuggerPluginPrivate::aboutToShutdown() disconnect(SessionManager::instance(), SIGNAL(startupProjectChanged(ProjectExplorer::Project*)), this, 0); + + m_mainWindow->saveCurrentPerspective(); + delete m_mainWindow; + m_mainWindow = 0; + + // removeObject leads to aboutToRemoveObject, which leads to + // ModeManager::aboutToRemove, which leads to the mode manager + // removing the mode's widget from the stackwidget + // (currently by index, but possibly the stackwidget resets the + // parent and stuff on the widget) + m_plugin->removeObject(m_mode); + + delete m_modeWindow; + m_modeWindow = 0; + + delete m_mode; + m_mode = 0; } void updateState(DebuggerEngine *engine) @@ -3214,7 +3217,6 @@ IPlugin::ShutdownFlag DebuggerPlugin::aboutToShutdown() { removeObject(this); dd->aboutToShutdown(); - dd->m_mainWindow->saveCurrentPerspective(); return SynchronousShutdown; } diff --git a/src/plugins/valgrind/callgrindtool.cpp b/src/plugins/valgrind/callgrindtool.cpp index d13659146e2..f72c3f1bbc2 100644 --- a/src/plugins/valgrind/callgrindtool.cpp +++ b/src/plugins/valgrind/callgrindtool.cpp @@ -503,7 +503,6 @@ CallgrindTool::CallgrindTool(QObject *parent) CallgrindTool::~CallgrindTool() { qDeleteAll(m_textMarks); - doClear(false); } void CallgrindTool::slotGoToOverview()