Clang: Do not call DocumentManager::modifiedDocuments() from worker thread

This is unsafe.

Change-Id: I8ac075a7289afa0d84785e37b1325d186a153000
Reviewed-by: Erik Verbruggen <erik.verbruggen@theqtcompany.com>

src/plugins/clangcodemodel/clangcompletionassistinterface.cpp

@@ -33,6 +33,7 @@
 #include "clangutils.h"
 
 #include <cpptools/cppmodelmanager.h>
+#include <cpptools/cpptoolsreuse.h>
 #include <cpptools/cppworkingcopy.h>
 
 #include <texteditor/texteditor.h>
@@ -58,7 +59,9 @@ ClangCompletionAssistInterface::ClangCompletionAssistInterface(
     , m_languageFeatures(features)
     , m_textEditorWidget(textEditorWidget)
 {
-    m_unsavedFiles = Utils::createUnsavedFiles(CppTools::CppModelManager::instance()->workingCopy());
+    m_unsavedFiles = Utils::createUnsavedFiles(
+                CppTools::CppModelManager::instance()->workingCopy(),
+                CppTools::modifiedFiles());
 }
 
 bool ClangCompletionAssistInterface::objcEnabled() const

src/plugins/clangcodemodel/clangeditordocumentparser.cpp

@@ -88,7 +88,7 @@ ClangEditorDocumentParser::ClangEditorDocumentParser(const QString &filePath)
 {
 }
 
-void ClangEditorDocumentParser::updateHelper(CppTools::WorkingCopy workingCopy)
+void ClangEditorDocumentParser::updateHelper(const BaseEditorDocumentParser::InMemoryInfo &info)
 {
     QTC_ASSERT(m_marker, return);
 
@@ -107,7 +107,8 @@ void ClangEditorDocumentParser::updateHelper(CppTools::WorkingCopy workingCopy)
     QMutexLocker lock(m_marker->mutex());
     m_marker->setFileName(filePath());
     m_marker->setCompilationOptions(options);
-    const Internal::UnsavedFiles unsavedFiles = Utils::createUnsavedFiles(workingCopy);
+    const Internal::UnsavedFiles unsavedFiles = Utils::createUnsavedFiles(info.workingCopy,
+                                                                          info.modifiedFiles);
     m_marker->reparse(unsavedFiles);
     qCDebug(log) << "Reparse took" << t.elapsed() << "ms.";
 }

src/plugins/clangcodemodel/clangeditordocumentparser.h

@@ -51,7 +51,7 @@ public:
     SemanticMarker::Ptr semanticMarker() const;
 
 private:
-    void updateHelper(CppTools::WorkingCopy workingCopy) override;
+    void updateHelper(const BaseEditorDocumentParser::InMemoryInfo &info) override;
 
     SemanticMarker::Ptr m_marker;
 };

src/plugins/clangcodemodel/clangeditordocumentprocessor.cpp

@@ -144,9 +144,6 @@ ClangEditorDocumentProcessor::~ClangEditorDocumentProcessor()
 void ClangEditorDocumentProcessor::run()
 {
     // Run clang parser
-    const CppTools::WorkingCopy workingCopy
-        = CppTools::CppModelManager::instance()->workingCopy();
-
     disconnect(&m_parserWatcher, &QFutureWatcher<void>::finished,
                this, &ClangEditorDocumentProcessor::onParserFinished);
     m_parserWatcher.cancel();
@@ -155,7 +152,9 @@ void ClangEditorDocumentProcessor::run()
     m_parserRevision = revision();
     connect(&m_parserWatcher, &QFutureWatcher<void>::finished,
             this, &ClangEditorDocumentProcessor::onParserFinished);
-    const QFuture<void> future = QtConcurrent::run(&runParser, parser(), workingCopy);
+    const QFuture<void> future = QtConcurrent::run(&runParser,
+                                                   parser(),
+                                                   ClangEditorDocumentParser::InMemoryInfo(true));
     m_parserWatcher.setFuture(future);
 
     // Run builtin processor

src/plugins/clangcodemodel/clangutils.cpp

@@ -32,7 +32,6 @@
 
 #include <clang-c/Index.h>
 
-#include <coreplugin/documentmanager.h>
 #include <coreplugin/icore.h>
 #include <coreplugin/idocument.h>
 
@@ -58,15 +57,9 @@ namespace Utils {
 
 Q_LOGGING_CATEGORY(verboseRunLog, "qtc.clangcodemodel.verboserun")
 
-UnsavedFiles createUnsavedFiles(WorkingCopy workingCopy)
+UnsavedFiles createUnsavedFiles(const WorkingCopy &workingCopy,
+                                const ::Utils::FileNameList &modifiedFiles)
 {
-    // TODO: change the modelmanager to hold one working copy, and amend it every time we ask for one.
-    // TODO: Reason: the UnsavedFile needs a QByteArray.
-
-    QSet< ::Utils::FileName> modifiedFiles;
-    foreach (IDocument *doc, DocumentManager::modifiedDocuments())
-        modifiedFiles.insert(doc->filePath());
-
     UnsavedFiles result;
     QHashIterator< ::Utils::FileName, QPair<QByteArray, unsigned> > wcIter = workingCopy.iterator();
     while (wcIter.hasNext()) {

src/plugins/clangcodemodel/clangutils.h

@@ -43,7 +43,9 @@ namespace Utils {
 
 Q_DECLARE_LOGGING_CATEGORY(verboseRunLog)
 
-ClangCodeModel::Internal::UnsavedFiles createUnsavedFiles(CppTools::WorkingCopy workingCopy);
+ClangCodeModel::Internal::UnsavedFiles createUnsavedFiles(
+        const CppTools::WorkingCopy &workingCopy,
+        const ::Utils::FileNameList &modifiedFiles);
 
 QStringList createClangOptions(const CppTools::ProjectPart::Ptr &pPart,
                                CppTools::ProjectFile::Kind fileKind);

src/plugins/cpptools/baseeditordocumentparser.cpp

@@ -31,7 +31,7 @@
 #include "baseeditordocumentparser.h"
 #include "baseeditordocumentprocessor.h"
 
-#include "cppworkingcopy.h"
+#include "cpptoolsreuse.h"
 #include "editordocumenthandle.h"
 
 namespace CppTools {
@@ -81,10 +81,10 @@ void BaseEditorDocumentParser::setConfiguration(const Configuration &configurati
     m_configuration = configuration;
 }
 
-void BaseEditorDocumentParser::update(WorkingCopy workingCopy)
+void BaseEditorDocumentParser::update(const InMemoryInfo &info)
 {
     QMutexLocker locker(&m_updateIsRunning);
-    updateHelper(workingCopy);
+    updateHelper(info);
 }
 
 BaseEditorDocumentParser::State BaseEditorDocumentParser::state() const
@@ -147,4 +147,11 @@ ProjectPart::Ptr BaseEditorDocumentParser::determineProjectPart(const QString &f
     return projectPart;
 }
 
+BaseEditorDocumentParser::InMemoryInfo::InMemoryInfo(bool withModifiedFiles)
+    : workingCopy(CppTools::CppModelManager::instance()->workingCopy())
+{
+    if (withModifiedFiles)
+        modifiedFiles = CppTools::modifiedFiles();
+}
+
 } // namespace CppTools

src/plugins/cpptools/baseeditordocumentparser.h

@@ -33,6 +33,7 @@
 
 #include "cppmodelmanager.h"
 #include "cpptools_global.h"
+#include "cppworkingcopy.h"
 
 #include <QObject>
 
@@ -59,7 +60,13 @@ public:
     Configuration configuration() const;
     void setConfiguration(const Configuration &configuration);
 
-    void update(WorkingCopy workingCopy);
+    struct CPPTOOLS_EXPORT InMemoryInfo {
+        InMemoryInfo(bool withModifiedFiles);
+
+        WorkingCopy workingCopy;
+        Utils::FileNameList modifiedFiles;
+    };
+    void update(const InMemoryInfo &info);
 
     ProjectPart::Ptr projectPart() const;
 
@@ -78,7 +85,7 @@ protected:
     mutable QMutex m_stateAndConfigurationMutex;
 
 private:
-    virtual void updateHelper(WorkingCopy workingCopy) = 0;
+    virtual void updateHelper(const InMemoryInfo &inMemoryInfo) = 0;
 
     const QString m_filePath;
     Configuration m_configuration;

src/plugins/cpptools/baseeditordocumentprocessor.cpp

@@ -31,6 +31,7 @@
 #include "baseeditordocumentprocessor.h"
 #include "cppworkingcopy.h"
 
+#include "cpptoolsreuse.h"
 #include "editordocumenthandle.h"
 
 #include <utils/qtcassert.h>
@@ -118,7 +119,7 @@ QList<QTextEdit::ExtraSelection> BaseEditorDocumentProcessor::toTextEditorSelect
 
 void BaseEditorDocumentProcessor::runParser(QFutureInterface<void> &future,
                                             BaseEditorDocumentParser *parser,
-                                            WorkingCopy workingCopy)
+                                            BaseEditorDocumentParser::InMemoryInfo info)
 {
     future.setProgressRange(0, 1);
     if (future.isCanceled()) {
@@ -126,7 +127,7 @@ void BaseEditorDocumentProcessor::runParser(QFutureInterface<void> &future,
         return;
     }
 
-    parser->update(workingCopy);
+    parser->update(info);
     CppModelManager::instance()
         ->finishedRefreshingSourceFiles(QSet<QString>() << parser->filePath());
 

src/plugins/cpptools/baseeditordocumentprocessor.h

@@ -86,7 +86,7 @@ protected:
 
     static void runParser(QFutureInterface<void> &future,
                           CppTools::BaseEditorDocumentParser *parser,
-                          CppTools::WorkingCopy workingCopy);
+                          BaseEditorDocumentParser::InMemoryInfo info);
 
     // Convenience
     QString filePath() const { return m_baseTextDocument->filePath().toString(); }

src/plugins/cpptools/builtineditordocumentparser.cpp

@@ -44,16 +44,17 @@ BuiltinEditorDocumentParser::BuiltinEditorDocumentParser(const QString &filePath
     qRegisterMetaType<CPlusPlus::Snapshot>("CPlusPlus::Snapshot");
 }
 
-void BuiltinEditorDocumentParser::updateHelper(WorkingCopy workingCopy)
+void BuiltinEditorDocumentParser::updateHelper(const InMemoryInfo &info)
 {
+    if (filePath().isEmpty())
+        return;
+
     const Configuration baseConfig = configuration();
     const bool releaseSourceAndAST_ = releaseSourceAndAST();
 
     State baseState = state();
     ExtraState state = extraState();
-
-    if (filePath().isEmpty())
-        return;
+    WorkingCopy workingCopy = info.workingCopy;
 
     bool invalidateSnapshot = false, invalidateConfig = false, editorDefinesChanged_ = false;
 

src/plugins/cpptools/builtineditordocumentparser.h

@@ -64,7 +64,7 @@ public:
     static BuiltinEditorDocumentParser *get(const QString &filePath);
 
 private:
-    void updateHelper(WorkingCopy workingCopy) override;
+    void updateHelper(const InMemoryInfo &info) override;
     void addFileAndDependencies(CPlusPlus::Snapshot *snapshot,
                                 QSet<Utils::FileName> *toRemove,
                                 const Utils::FileName &fileName) const;

src/plugins/cpptools/builtineditordocumentprocessor.cpp

@@ -43,8 +43,8 @@
 #include <cplusplus/CppDocument.h>
 #include <cplusplus/SimpleLexer.h>
 
-#include <utils/QtConcurrentTools>
 #include <utils/qtcassert.h>
+#include <utils/runextensions.h>
 
 #include <QLoggingCategory>
 
@@ -166,7 +166,9 @@ BuiltinEditorDocumentProcessor::~BuiltinEditorDocumentProcessor()
 
 void BuiltinEditorDocumentProcessor::run()
 {
-    m_parserFuture = QtConcurrent::run(&runParser, parser(), CppTools::CppModelManager::instance()->workingCopy());
+    m_parserFuture = QtConcurrent::run(&runParser,
+                                       parser(),
+                                       BuiltinEditorDocumentParser::InMemoryInfo(false));
 }
 
 BaseEditorDocumentParser *BuiltinEditorDocumentProcessor::parser()

src/plugins/cpptools/builtinindexingsupport.cpp

@@ -162,7 +162,7 @@ void indexFindErrors(QFutureInterface<void> &future, const ParseParams params)
         // Parse the file as precisely as possible
         BuiltinEditorDocumentParser parser(file);
         parser.setReleaseSourceAndAST(false);
-        parser.update(params.workingCopy);
+        parser.update(BuiltinEditorDocumentParser::InMemoryInfo(false));
         CPlusPlus::Document::Ptr document = parser.document();
         QTC_ASSERT(document, return);
 

src/plugins/cpptools/cppcompletionassist.cpp

@@ -2188,7 +2188,7 @@ void CppCompletionAssistInterface::getCppSpecifics() const
     m_gotCppSpecifics = true;
 
     if (BuiltinEditorDocumentParser *parser = BuiltinEditorDocumentParser::get(fileName())) {
-        parser->update(m_workingCopy);
+        parser->update(BuiltinEditorDocumentParser::InMemoryInfo(false));
         m_snapshot = parser->snapshot();
         m_headerPaths = parser->headerPaths();
         if (Document::Ptr document = parser->document())

src/plugins/cpptools/cppmodelmanager_test.cpp

@@ -915,7 +915,7 @@ void CppToolsPlugin::test_modelmanager_precompiled_headers()
         BaseEditorDocumentParser::Configuration config = parser->configuration();
         config.usePrecompiledHeaders = true;
         parser->setConfiguration(config);
-        parser->update(mm->workingCopy());
+        parser->update(BuiltinEditorDocumentParser::InMemoryInfo(false));
 
         // Check if defines from pch are considered
         Document::Ptr document = mm->document(fileName);
@@ -998,8 +998,7 @@ void CppToolsPlugin::test_modelmanager_defines_per_editor()
         BaseEditorDocumentParser::Configuration config = parser->configuration();
         config.editorDefines = editorDefines.toUtf8();
         parser->setConfiguration(config);
-
-        parser->update(mm->workingCopy());
+        parser->update(BuiltinEditorDocumentParser::InMemoryInfo(false));
 
         Document::Ptr doc = mm->document(main1File);
         QCOMPARE(nameOfFirstDeclaration(doc), firstDeclarationName);

src/plugins/cpptools/cpptoolsreuse.cpp

@@ -32,6 +32,7 @@
 
 #include "cpptoolsplugin.h"
 
+#include <coreplugin/documentmanager.h>
 #include <coreplugin/editormanager/editormanager.h>
 #include <coreplugin/idocument.h>
 #include <texteditor/convenience.h>
@@ -287,4 +288,13 @@ bool skipFileDueToSizeLimit(const QFileInfo &fileInfo, int limitInMB)
     return false;
 }
 
+Utils::FileNameList modifiedFiles()
+{
+    Utils::FileNameList files;
+    foreach (Core::IDocument *doc, Core::DocumentManager::modifiedDocuments())
+        files.append(doc->filePath());
+    files.removeDuplicates();
+    return files;
+}
+
 } // CppTools

src/plugins/cpptools/cpptoolsreuse.h

@@ -44,6 +44,10 @@ class QStringRef;
 class QTextCursor;
 QT_END_NAMESPACE
 
+namespace Utils {
+class FileNameList;
+} // namespace Utils
+
 namespace CPlusPlus {
 class Macro;
 class Symbol;
@@ -52,6 +56,8 @@ class LookupContext;
 
 namespace CppTools {
 
+Utils::FileNameList CPPTOOLS_EXPORT modifiedFiles();
+
 void CPPTOOLS_EXPORT moveCursorToEndOfIdentifier(QTextCursor *tc);
 void CPPTOOLS_EXPORT moveCursorToStartOfIdentifier(QTextCursor *tc);