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 <christian.kandeler@qt.io>

src/plugins/classview/classviewmanager.cpp

@@ -90,12 +90,8 @@ static Manager *managerInstance = nullptr;
 class ManagerPrivate
 {
 public:
-    //! code state/changes parser
+    Parser *m_parser = nullptr;
-    Parser parser;
+    QThread m_parserThread;
-
-    //! separate thread for the parser
-    QThread 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->m_parserThread.start();
-    d->parserThread.start();
 }
 
 Manager::~Manager()
 {
-    QMetaObject::invokeMethod(&d->parser, &Parser::aboutToShutdown, Qt::BlockingQueuedConnection);
+    d->m_parserThread.quit();
-    d->parserThread.quit();
+    d->m_parserThread.wait();
-    d->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]() {
+        QMetaObject::invokeMethod(d->m_parser, [this, doc]() {
-            d->parser.parseDocument(doc); }, Qt::QueuedConnection);
+            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<QVariant> &list)
 
 void Manager::setFlatMode(bool flat)
 {
-    QMetaObject::invokeMethod(&d->parser, [this, flat]() {
+    QMetaObject::invokeMethod(d->m_parser, [this, flat]() {
-        d->parser.setFlatMode(flat);
+        d->m_parser->setFlatMode(flat);
     }, Qt::QueuedConnection);
 }
 

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
 }
 

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);