CppEditor: Use Utils::SynchronizedValue for project data

Change-Id: I67c07c2f5f5f7e8bb6b10d4f492ad062f82977be
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/plugins/cppeditor/cppmodelmanager.cpp

@@ -69,6 +69,7 @@
 #include <utils/process.h>
 #include <utils/qtcassert.h>
 #include <utils/savefile.h>
+#include <utils/synchronizedvalue.h>
 #include <utils/temporarydirectory.h>
 
 #include <QAction>
@@ -169,16 +170,20 @@ public:
     Snapshot m_snapshot;
 
     // Project integration
-    QReadWriteLock m_projectLock;
+    struct SyncedProjectData
+    {
         QHash<Project *, ProjectData> m_projectData;
         QMap<FilePath, QList<ProjectPart::ConstPtr>> m_fileToProjectParts;
         QMap<QString, ProjectPart::ConstPtr> m_projectPartIdToProjectProjectPart;
 
         // The members below are cached/(re)calculated from the projects and/or their project parts
-    bool m_dirty;
+        bool m_dirty{true};
         FilePaths m_projectFiles;
         HeaderPaths m_headerPaths;
         Macros m_definedMacros;
+    };
+
+    Utils::SynchronizedValue<SyncedProjectData> m_lockedProjectData;
 
     // Editor integration
     mutable QMutex m_cppEditorDocumentsMutex;
@@ -215,6 +220,12 @@ public:
     std::unique_ptr<ILocatorFilter> m_currentDocumentFilter;
 
     QList<Document::DiagnosticMessage> m_diagnosticMessages;
+
+    static void recalculateProjectPartMappings(SyncedProjectData &ld);
+    static void ensureUpdated(SyncedProjectData &ld);
+    static Utils::FilePaths internalProjectFiles(SyncedProjectData &ld);
+    static ProjectExplorer::HeaderPaths internalHeaderPaths(SyncedProjectData &ld);
+    static ProjectExplorer::Macros internalDefinedMacros(SyncedProjectData &ld);
 };
 
 static CppModelManagerPrivate *d;
@@ -989,8 +1000,6 @@ CppModelManager::CppModelManager()
     d->m_findReferences = new CppFindReferences(this);
     d->m_indexerEnabled = qtcEnvironmentVariable("QTC_NO_CODE_INDEXER") != "1";
 
-    d->m_dirty = true;
-
     d->m_delayedGcTimer.setObjectName(QLatin1String("CppModelManager::m_delayedGcTimer"));
     d->m_delayedGcTimer.setSingleShot(true);
     connect(&d->m_delayedGcTimer, &QTimer::timeout, this, &CppModelManager::GC);
@@ -1071,22 +1080,21 @@ bool CppModelManager::replaceDocument(Document::Ptr newDoc)
     return true;
 }
 
-/// Make sure that m_projectLock is locked for writing when calling this.
+void CppModelManagerPrivate::ensureUpdated(SyncedProjectData &ld)
-void CppModelManager::ensureUpdated()
 {
-    if (!d->m_dirty)
+    if (!ld.m_dirty)
         return;
 
-    d->m_projectFiles = internalProjectFiles();
+    ld.m_projectFiles = internalProjectFiles(ld);
-    d->m_headerPaths = internalHeaderPaths();
+    ld.m_headerPaths = internalHeaderPaths(ld);
-    d->m_definedMacros = internalDefinedMacros();
+    ld.m_definedMacros = internalDefinedMacros(ld);
-    d->m_dirty = false;
+    ld.m_dirty = false;
 }
 
-FilePaths CppModelManager::internalProjectFiles()
+FilePaths CppModelManagerPrivate::internalProjectFiles(SyncedProjectData &ld)
 {
     FilePaths files;
-    for (const ProjectData &projectData : std::as_const(d->m_projectData)) {
+    for (const ProjectData &projectData : std::as_const(ld.m_projectData)) {
         for (const ProjectPart::ConstPtr &part : projectData.projectInfo->projectParts()) {
             for (const ProjectFile &file : part->files)
                 files += file.path;
@@ -1096,10 +1104,10 @@ FilePaths CppModelManager::internalProjectFiles()
     return files;
 }
 
-HeaderPaths CppModelManager::internalHeaderPaths()
+HeaderPaths CppModelManagerPrivate::internalHeaderPaths(SyncedProjectData &ld)
 {
     HeaderPaths headerPaths;
-    for (const ProjectData &projectData: std::as_const(d->m_projectData)) {
+    for (const ProjectData &projectData : std::as_const(ld.m_projectData)) {
         for (const ProjectPart::ConstPtr &part : projectData.projectInfo->projectParts()) {
             for (const HeaderPath &path : part->headerPaths) {
                 HeaderPath hp(QDir::cleanPath(path.path), path.type);
@@ -1120,11 +1128,11 @@ static void addUnique(const Macros &newMacros, Macros &macros,
     }
 }
 
-Macros CppModelManager::internalDefinedMacros()
+Macros CppModelManagerPrivate::internalDefinedMacros(SyncedProjectData &ld)
 {
     Macros macros;
     QSet<ProjectExplorer::Macro> alreadyIn;
-    for (const ProjectData &projectData : std::as_const(d->m_projectData)) {
+    for (const ProjectData &projectData : std::as_const(ld.m_projectData)) {
         for (const ProjectPart::ConstPtr &part : projectData.projectInfo->projectParts()) {
             addUnique(part->toolChainMacros, macros, alreadyIn);
             addUnique(part->projectMacros, macros, alreadyIn);
@@ -1351,15 +1359,19 @@ QFuture<void> CppModelManager::updateSourceFiles(const QSet<FilePath> &sourceFil
 
 ProjectInfoList CppModelManager::projectInfos()
 {
-    QReadLocker locker(&d->m_projectLock);
+    return Utils::transform<QList<ProjectInfo::ConstPtr>>(d->m_lockedProjectData.readLocked()
-    return Utils::transform<QList<ProjectInfo::ConstPtr>>(d->m_projectData,
+                                                              ->m_projectData,
-            [](const ProjectData &d) { return d.projectInfo; });
+                                                          [](const ProjectData &d) {
+                                                              return d.projectInfo;
+                                                          });
 }
 
 ProjectInfo::ConstPtr CppModelManager::projectInfo(Project *project)
 {
-    QReadLocker locker(&d->m_projectLock);
+    return d->m_lockedProjectData.get<ProjectInfo::ConstPtr>(
-    return d->m_projectData.value(project).projectInfo;
+        [project](const CppModelManagerPrivate::SyncedProjectData &ld) {
+            return ld.m_projectData.value(project).projectInfo;
+        });
 }
 
 /// \brief Remove all files and their includes (recursively) of given ProjectInfo from the snapshot.
@@ -1460,18 +1472,17 @@ private:
     const QSet<FilePath> m_newSourceFiles;
 };
 
-/// Make sure that m_projectLock is locked for writing when calling this.
+void CppModelManagerPrivate::recalculateProjectPartMappings(SyncedProjectData &ld)
-void CppModelManager::recalculateProjectPartMappings()
 {
-    d->m_projectPartIdToProjectProjectPart.clear();
+    ld.m_projectPartIdToProjectProjectPart.clear();
-    d->m_fileToProjectParts.clear();
+    ld.m_fileToProjectParts.clear();
-    for (const ProjectData &projectData : std::as_const(d->m_projectData)) {
+    for (const ProjectData &projectData : std::as_const(ld.m_projectData)) {
         for (const ProjectPart::ConstPtr &projectPart : projectData.projectInfo->projectParts()) {
-            d->m_projectPartIdToProjectProjectPart[projectPart->id()] = projectPart;
+            ld.m_projectPartIdToProjectProjectPart[projectPart->id()] = projectPart;
             for (const ProjectFile &cxxFile : projectPart->files) {
-                d->m_fileToProjectParts[cxxFile.path].append(projectPart);
+                ld.m_fileToProjectParts[cxxFile.path].append(projectPart);
                 if (FilePath canonical = cxxFile.path.canonicalPath(); canonical != cxxFile.path)
-                    d->m_fileToProjectParts[canonical].append(projectPart);
+                    ld.m_fileToProjectParts[canonical].append(projectPart);
             }
         }
     }
@@ -1483,12 +1494,15 @@ void CppModelManagerPrivate::setupWatcher(const QFuture<void> &future, Project *
                                           ProjectData *projectData, CppModelManager *q)
 {
     projectData->indexer = new QFutureWatcher<void>(q);
-    const auto handleFinished = [this, project, watcher = projectData->indexer, q] {
+    const auto handleFinished = [project, watcher = projectData->indexer, q] {
-        if (const auto it = m_projectData.find(project);
+        d->m_lockedProjectData.write([watcher, project](auto &ld) {
-                it != m_projectData.end() && it->indexer == watcher) {
+            const auto it = ld.m_projectData.find(project);
+            if (it != ld.m_projectData.end() && it->indexer == watcher) {
                 it->indexer = nullptr;
                 it->fullyIndexed = !watcher->isCanceled();
             }
+        });
+
         watcher->disconnect(q);
         watcher->deleteLater();
     };
@@ -1542,17 +1556,21 @@ QFuture<void> CppModelManager::updateProjectInfo(const ProjectInfo::ConstPtr &ne
         return {};
 
     ProjectData *projectData = nullptr;
-    { // Only hold the lock for a limited scope, so the dumping afterwards does not deadlock.
-        QWriteLocker projectLocker(&d->m_projectLock);
 
+    d->m_lockedProjectData.write([&newProjectInfo,
+                                  project,
+                                  &filesToReindex,
+                                  &removedProjectParts,
+                                  &filesRemoved,
+                                  &projectData](auto &ld) {
         const QSet<FilePath> newSourceFiles = newProjectInfo->sourceFiles();
 
         // Check if we can avoid a full reindexing
-        const auto it = d->m_projectData.find(project);
+        const auto it = ld.m_projectData.find(project);
-        if (it != d->m_projectData.end() && it->projectInfo && it->fullyIndexed) {
+        if (it != ld.m_projectData.end() && it->projectInfo && it->fullyIndexed) {
             ProjectInfoComparer comparer(*it->projectInfo, *newProjectInfo);
             if (comparer.configurationOrFilesChanged()) {
-                d->m_dirty = true;
+                ld.m_dirty = true;
 
                 // If the project configuration changed, do a full reindexing
                 if (comparer.configurationChanged()) {
@@ -1587,22 +1605,22 @@ QFuture<void> CppModelManager::updateProjectInfo(const ProjectInfo::ConstPtr &ne
 
         // A new project was opened/created, do a full indexing
         } else {
-            d->m_dirty = true;
+            ld.m_dirty = true;
             filesToReindex.unite(newSourceFiles);
         }
 
         // Update Project/ProjectInfo and File/ProjectPart table
-        if (it != d->m_projectData.end()) {
+        if (it != ld.m_projectData.end()) {
             if (it->indexer)
                 it->indexer->cancel();
             it->projectInfo = newProjectInfo;
             it->fullyIndexed = false;
         }
-        projectData = it == d->m_projectData.end()
+        projectData = it == ld.m_projectData.end() ? &(
-                ? &(d->m_projectData[project] = ProjectData{newProjectInfo, nullptr, false})
+                          ld.m_projectData[project] = ProjectData{newProjectInfo, nullptr, false})
                                                    : &(*it);
-        recalculateProjectPartMappings();
+        CppModelManagerPrivate::recalculateProjectPartMappings(ld);
-    } // Locker scope
+    });
 
     // If requested, dump everything we got
     if (DumpProjectInfo)
@@ -1639,25 +1657,38 @@ QFuture<void> CppModelManager::updateProjectInfo(const ProjectInfo::ConstPtr &ne
 
 ProjectPart::ConstPtr CppModelManager::projectPartForId(const QString &projectPartId)
 {
-    QReadLocker locker(&d->m_projectLock);
+    return d->m_lockedProjectData.get<ProjectPart::ConstPtr>(
-    return d->m_projectPartIdToProjectProjectPart.value(projectPartId);
+        [projectPartId](const CppModelManagerPrivate::SyncedProjectData &ld) {
+            return ld.m_projectPartIdToProjectProjectPart.value(projectPartId);
+        });
 }
 
 QList<ProjectPart::ConstPtr> CppModelManager::projectPart(const FilePath &fileName)
 {
-    {
+    std::optional<QList<ProjectPart::ConstPtr>> result;
-        QReadLocker locker(&d->m_projectLock);
+
-        auto it = d->m_fileToProjectParts.constFind(fileName);
+    d->m_lockedProjectData.read(
-        if (it != d->m_fileToProjectParts.constEnd())
+        [&fileName, &result](const CppModelManagerPrivate::SyncedProjectData &sd) {
-            return it.value();
+            const auto it = sd.m_fileToProjectParts.constFind(fileName);
-    }
+            if (it != sd.m_fileToProjectParts.constEnd())
+                result = *it;
+        });
+
+    if (result)
+        return *result;
+
     const FilePath canonicalPath = fileName.canonicalPath();
-    QWriteLocker locker(&d->m_projectLock);
+
-    const auto it = d->m_fileToProjectParts.constFind(canonicalPath);
+    d->m_lockedProjectData.write(
-    if (it == d->m_fileToProjectParts.constEnd())
+        [&fileName, &canonicalPath, &result](CppModelManagerPrivate::SyncedProjectData &sd) {
-        return {};
+            const auto it = sd.m_fileToProjectParts.constFind(canonicalPath);
-    d->m_fileToProjectParts.insert(fileName, it.value());
+            if (it == sd.m_fileToProjectParts.constEnd())
-    return it.value();
+                return;
+            sd.m_fileToProjectParts.insert(fileName, it.value());
+            result = it.value();
+        });
+
+    return result.value_or(QList<ProjectPart::ConstPtr>{});
 }
 
 QList<ProjectPart::ConstPtr> CppModelManager::projectPartFromDependencies(
@@ -1713,8 +1744,7 @@ void CppModelManager::emitAbstractEditorSupportRemoved(const QString &filePath)
 
 void CppModelManager::onProjectAdded(Project *)
 {
-    QWriteLocker locker(&d->m_projectLock);
+    d->m_lockedProjectData.writeLocked()->m_dirty = true;
-    d->m_dirty = true;
 }
 
 void CppModelManager::delayedGC()
@@ -1735,17 +1765,17 @@ void CppModelManager::onAboutToRemoveProject(Project *project)
 {
     QStringList idsOfRemovedProjectParts;
 
-    {
+    d->m_lockedProjectData.write([project, &idsOfRemovedProjectParts](
-        QWriteLocker locker(&d->m_projectLock);
+                                     CppModelManagerPrivate::SyncedProjectData &sd) {
-        d->m_dirty = true;
+        sd.m_dirty = true;
-        const QStringList projectPartsIdsBefore = d->m_projectPartIdToProjectProjectPart.keys();
+        const QStringList projectPartsIdsBefore = sd.m_projectPartIdToProjectProjectPart.keys();
 
-        d->m_projectData.remove(project);
+        sd.m_projectData.remove(project);
-        recalculateProjectPartMappings();
+        CppModelManagerPrivate::recalculateProjectPartMappings(sd);
 
-        const QStringList projectPartsIdsAfter = d->m_projectPartIdToProjectProjectPart.keys();
+        const QStringList projectPartsIdsAfter = sd.m_projectPartIdToProjectProjectPart.keys();
         idsOfRemovedProjectParts = removedProjectParts(projectPartsIdsBefore, projectPartsIdsAfter);
-    }
+    });
 
     if (!idsOfRemovedProjectParts.isEmpty())
         emit m_instance->projectPartsRemoved(idsOfRemovedProjectParts);
@@ -1758,11 +1788,8 @@ void CppModelManager::onActiveProjectChanged(Project *project)
     if (!project)
         return; // Last project closed.
 
-    {
+    if (!d->m_lockedProjectData.readLocked()->m_projectData.contains(project))
-        QReadLocker locker(&d->m_projectLock);
-        if (!d->m_projectData.contains(project))
         return; // Not yet known to us.
-    }
 
     updateCppEditorDocuments();
 }
@@ -2103,32 +2130,33 @@ CppIndexingSupport *CppModelManager::indexingSupport()
 
 FilePaths CppModelManager::projectFiles()
 {
-    QWriteLocker locker(&d->m_projectLock);
+    return d->m_lockedProjectData.update<FilePaths>(
-    ensureUpdated();
+        [](CppModelManagerPrivate::SyncedProjectData &sd) {
-
+            CppModelManagerPrivate::ensureUpdated(sd);
-    return d->m_projectFiles;
+            return sd.m_projectFiles;
+        });
 }
 
 HeaderPaths CppModelManager::headerPaths()
 {
-    QWriteLocker locker(&d->m_projectLock);
+    return d->m_lockedProjectData.update<HeaderPaths>(
-    ensureUpdated();
+        [](CppModelManagerPrivate::SyncedProjectData &sd) {
-
+            CppModelManagerPrivate::ensureUpdated(sd);
-    return d->m_headerPaths;
+            return sd.m_headerPaths;
+        });
 }
 
 void CppModelManager::setHeaderPaths(const HeaderPaths &headerPaths)
 {
-    QWriteLocker locker(&d->m_projectLock);
+    d->m_lockedProjectData.writeLocked()->m_headerPaths = headerPaths;
-    d->m_headerPaths = headerPaths;
 }
 
 Macros CppModelManager::definedMacros()
 {
-    QWriteLocker locker(&d->m_projectLock);
+    return d->m_lockedProjectData.update<Macros>([](CppModelManagerPrivate::SyncedProjectData &sd) {
-    ensureUpdated();
+        CppModelManagerPrivate::ensureUpdated(sd);
-
+        return sd.m_definedMacros;
-    return d->m_definedMacros;
+    });
 }
 
 void CppModelManager::enableGarbageCollector(bool enable)

src/plugins/cppeditor/cppmodelmanager.h

@@ -290,7 +290,6 @@ private:
     static void setupFallbackProjectPart();
 
     static void delayedGC();
-    static void recalculateProjectPartMappings();
 
     static void replaceSnapshot(const CPlusPlus::Snapshot &newSnapshot);
     static void removeFilesFromSnapshot(const QSet<Utils::FilePath> &removedFiles);
@@ -298,11 +297,6 @@ private:
 
     static WorkingCopy buildWorkingCopyList();
 
-    static void ensureUpdated();
-    static Utils::FilePaths internalProjectFiles();
-    static ProjectExplorer::HeaderPaths internalHeaderPaths();
-    static ProjectExplorer::Macros internalDefinedMacros();
-
     static void dumpModelManagerConfiguration(const QString &logFileId);
     static void initCppTools();
 };