From 72f727b6aca14e16961f6bcb20b3fc06c2cbfafc Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Tue, 16 Feb 2021 17:44:28 +0100 Subject: [PATCH] ClassView: Delete Parser object from inside parser's thread The Parser object is being moved to parser's thread inside Manager constructor. However, when destructor of Manager is being called, we delete the Parser from inside the main thread. According to QThread documentation we should delete object (which have been moved to another thread) from inside the object's current thread. So in case of Parser, we should delete it from the parser's thread. In order to fix it, we create Parser object dynamically and connect finished signal of the parser's thread to the parser's deleteLater(). Since now the parser is being deleted in parser's thread we don't need a special handling for stopping the timer object inside the parser's thread, as its destructor will also be called from inside parser's thread. Task-number: QTCREATORBUG-25317 Change-Id: I28dee2c3db5cf8329a9578e7a85952e8a85850d3 Reviewed-by: Christian Kandeler --- src/plugins/classview/classviewmanager.cpp | 35 ++++++++++------------ src/plugins/classview/classviewparser.cpp | 9 +----- src/plugins/classview/classviewparser.h | 1 - 3 files changed, 17 insertions(+), 28 deletions(-) diff --git a/src/plugins/classview/classviewmanager.cpp b/src/plugins/classview/classviewmanager.cpp index 134e3314f92..5d157046519 100644 --- a/src/plugins/classview/classviewmanager.cpp +++ b/src/plugins/classview/classviewmanager.cpp @@ -90,12 +90,8 @@ static Manager *managerInstance = nullptr; class ManagerPrivate { public: - //! code state/changes parser - Parser parser; - - //! separate thread for the parser - QThread parserThread; - + Parser *m_parser = nullptr; + QThread m_parserThread; ParserTreeItem::ConstPtr m_root; //! Internal manager state. \sa Manager::state @@ -111,6 +107,9 @@ Manager::Manager(QObject *parent) : QObject(parent), d(new ManagerPrivate()) { + d->m_parser = new Parser(); + d->m_parser->moveToThread(&d->m_parserThread); + connect(&d->m_parserThread, &QThread::finished, d->m_parser, &QObject::deleteLater); managerInstance = this; // register - to be able send between signal/slots @@ -119,15 +118,13 @@ Manager::Manager(QObject *parent) initialize(); // start a separate thread for the parser - d->parser.moveToThread(&d->parserThread); - d->parserThread.start(); + d->m_parserThread.start(); } Manager::~Manager() { - QMetaObject::invokeMethod(&d->parser, &Parser::aboutToShutdown, Qt::BlockingQueuedConnection); - d->parserThread.quit(); - d->parserThread.wait(); + d->m_parserThread.quit(); + d->m_parserThread.wait(); delete d; managerInstance = nullptr; } @@ -237,7 +234,7 @@ void Manager::initialize() resetParser(); }); - connect(&d->parser, &Parser::treeRegenerated, + connect(d->m_parser, &Parser::treeRegenerated, this, [this](const ParserTreeItem::ConstPtr &root) { d->m_root = root; @@ -263,12 +260,12 @@ void Manager::initialize() if (d->disableCodeParser) return; - QMetaObject::invokeMethod(&d->parser, [this, doc]() { - d->parser.parseDocument(doc); }, Qt::QueuedConnection); + QMetaObject::invokeMethod(d->m_parser, [this, doc]() { + d->m_parser->parseDocument(doc); }, Qt::QueuedConnection); }, Qt::QueuedConnection); // connect(codeModelManager, &CppTools::CppModelManager::aboutToRemoveFiles, - &d->parser, &Parser::removeFiles, Qt::QueuedConnection); + d->m_parser, &Parser::removeFiles, Qt::QueuedConnection); } /*! @@ -305,7 +302,7 @@ void Manager::setState(bool state) void Manager::resetParser() { - QMetaObject::invokeMethod(&d->parser, &Parser::resetDataToCurrentState, Qt::QueuedConnection); + QMetaObject::invokeMethod(d->m_parser, &Parser::resetDataToCurrentState, Qt::QueuedConnection); } /*! @@ -335,7 +332,7 @@ void Manager::onProjectListChanged() if (!state()) return; - QMetaObject::invokeMethod(&d->parser, &Parser::requestCurrentState, Qt::QueuedConnection); + QMetaObject::invokeMethod(d->m_parser, &Parser::requestCurrentState, Qt::QueuedConnection); } /*! @@ -393,8 +390,8 @@ void Manager::gotoLocations(const QList &list) void Manager::setFlatMode(bool flat) { - QMetaObject::invokeMethod(&d->parser, [this, flat]() { - d->parser.setFlatMode(flat); + QMetaObject::invokeMethod(d->m_parser, [this, flat]() { + d->m_parser->setFlatMode(flat); }, Qt::QueuedConnection); } diff --git a/src/plugins/classview/classviewparser.cpp b/src/plugins/classview/classviewparser.cpp index 01bdb278bb9..d4b8716751f 100644 --- a/src/plugins/classview/classviewparser.cpp +++ b/src/plugins/classview/classviewparser.cpp @@ -88,7 +88,6 @@ public: CPlusPlus::Document::Ptr document(const QString &fileName) const; QTimer timer; - bool m_shuttingDown = false; struct DocumentCache { unsigned treeRevision = 0; @@ -160,12 +159,6 @@ void Parser::setFlatMode(bool flatMode) requestCurrentState(); } -void Parser::aboutToShutdown() -{ - d->m_shuttingDown = true; - d->timer.stop(); -} - /*! Parses the class and produces a new tree. @@ -331,7 +324,7 @@ void Parser::parseDocument(const CPlusPlus::Document::Ptr &doc) getParseDocumentTree(doc); - if (!d->timer.isActive() && !d->m_shuttingDown) + if (!d->timer.isActive()) d->timer.start(400); //! Delay in msecs before an update } diff --git a/src/plugins/classview/classviewparser.h b/src/plugins/classview/classviewparser.h index 65d8227c1d2..9249bb9f1ae 100644 --- a/src/plugins/classview/classviewparser.h +++ b/src/plugins/classview/classviewparser.h @@ -57,7 +57,6 @@ public: void resetDataToCurrentState(); void parseDocument(const CPlusPlus::Document::Ptr &doc); void setFlatMode(bool flat); - void aboutToShutdown(); signals: void treeRegenerated(const ParserTreeItem::ConstPtr &root);