feedc0de/qt-creator
1
0
Fork 0
forked from qt-creator/qt-creator
Code Pull Requests Activity

CppTools: Replace a QMutex with a QReadWriteLock

Browse Source 
... in CppModelManager. Not all accesses are mutually exclusive.

Change-Id: I87fb50db35c5fdfa401f08ad221ca053911eac07
Reviewed-by: Jarek Kobus <jaroslaw.kobus@qt.io>
This commit is contained in:
Christian Kandeler
2021-08-20 11:15:06 +02:00
parent 3b75ca96d6
commit 5d8551559a
1 changed files with 21 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
src/plugins/cpptools/cppmodelmanager.cpp
View File

@@ -77,9 +77,12 @@
#include <QDir> #include <QDir>
#include <QFutureWatcher> #include <QFutureWatcher>
#include <QMutexLocker> #include <QMutexLocker>
#include <QReadLocker>
#include <QReadWriteLock>
#include <QTextBlock> #include <QTextBlock>
#include <QThreadPool> #include <QThreadPool>
#include <QTimer> #include <QTimer>
#include <QWriteLocker>
#if defined(QTCREATOR_WITH_DUMP_AST) && defined(Q_CC_GNU) #if defined(QTCREATOR_WITH_DUMP_AST) && defined(Q_CC_GNU)
#define WITH_AST_DUMP #define WITH_AST_DUMP
@@ -163,7 +166,7 @@ public:
Snapshot m_snapshot; Snapshot m_snapshot;
// Project integration // Project integration
mutable QMutex m_projectMutex; QReadWriteLock m_projectLock;
QHash<ProjectExplorer::Project *, ProjectData> m_projectData; QHash<ProjectExplorer::Project *, ProjectData> m_projectData;
QMap<Utils::FilePath, QList<ProjectPart::Ptr> > m_fileToProjectParts; QMap<Utils::FilePath, QList<ProjectPart::Ptr> > m_fileToProjectParts;
QMap<QString, ProjectPart::Ptr> m_projectPartIdToProjectProjectPart; QMap<QString, ProjectPart::Ptr> m_projectPartIdToProjectProjectPart;
@@ -742,7 +745,7 @@ bool CppModelManager::replaceDocument(Document::Ptr newDoc)
return true; return true;
} }
/// Make sure that m_projectMutex is locked when calling this. /// Make sure that m_projectLock is locked for writing when calling this.
void CppModelManager::ensureUpdated() void CppModelManager::ensureUpdated()
{ {
if (!d->m_dirty) if (!d->m_dirty)
@@ -981,14 +984,14 @@ QFuture<void> CppModelManager::updateSourceFiles(const QSet<QString> &sourceFile
QList<ProjectInfo::Ptr> CppModelManager::projectInfos() const QList<ProjectInfo::Ptr> CppModelManager::projectInfos() const
{ {
QMutexLocker locker(&d->m_projectMutex); QReadLocker locker(&d->m_projectLock);
return Utils::transform<QList<ProjectInfo::Ptr>>(d->m_projectData, return Utils::transform<QList<ProjectInfo::Ptr>>(d->m_projectData,
[](const ProjectData &d) { return d.projectInfo; }); [](const ProjectData &d) { return d.projectInfo; });
} }
ProjectInfo::Ptr CppModelManager::projectInfo(ProjectExplorer::Project *project) const ProjectInfo::Ptr CppModelManager::projectInfo(ProjectExplorer::Project *project) const
{ {
QMutexLocker locker(&d->m_projectMutex); QReadLocker locker(&d->m_projectLock);
return d->m_projectData.value(project).projectInfo; return d->m_projectData.value(project).projectInfo;
} }
@@ -1089,7 +1092,7 @@ private:
const QSet<QString> m_newSourceFiles; const QSet<QString> m_newSourceFiles;
}; };
/// Make sure that m_projectMutex is locked when calling this. /// Make sure that m_projectLock is locked for writing when calling this.
void CppModelManager::recalculateProjectPartMappings() void CppModelManager::recalculateProjectPartMappings()
{ {
d->m_projectPartIdToProjectProjectPart.clear(); d->m_projectPartIdToProjectProjectPart.clear();
@@ -1169,8 +1172,8 @@ QFuture<void> CppModelManager::updateProjectInfo(const ProjectInfo::Ptr &newProj
return {}; return {};
ProjectData *projectData = nullptr; ProjectData *projectData = nullptr;
{ // Only hold the mutex for a limited scope, so the dumping afterwards does not deadlock. { // Only hold the lock for a limited scope, so the dumping afterwards does not deadlock.
QMutexLocker projectLocker(&d->m_projectMutex); QWriteLocker projectLocker(&d->m_projectLock);
const QSet<QString> newSourceFiles = newProjectInfo->sourceFiles(); const QSet<QString> newSourceFiles = newProjectInfo->sourceFiles();
@@ -1230,7 +1233,7 @@ QFuture<void> CppModelManager::updateProjectInfo(const ProjectInfo::Ptr &newProj
? &(d->m_projectData[project] = ProjectData{newProjectInfo, nullptr, false}) ? &(d->m_projectData[project] = ProjectData{newProjectInfo, nullptr, false})
: &(*it); : &(*it);
recalculateProjectPartMappings(); recalculateProjectPartMappings();
} // Mutex scope } // Locker scope
// If requested, dump everything we got // If requested, dump everything we got
if (DumpProjectInfo) if (DumpProjectInfo)
@@ -1260,7 +1263,6 @@ QFuture<void> CppModelManager::updateProjectInfo(const ProjectInfo::Ptr &newProj
// It's safe to do this here, as only the UI thread writes to the map and no other thread // It's safe to do this here, as only the UI thread writes to the map and no other thread
// uses the indexer value. // uses the indexer value.
// FIXME: Use a read/write lock instead of a mutex.
d->setupWatcher(indexingFuture, project, projectData, this); d->setupWatcher(indexingFuture, project, projectData, this);
return indexingFuture; return indexingFuture;
@@ -1268,12 +1270,13 @@ QFuture<void> CppModelManager::updateProjectInfo(const ProjectInfo::Ptr &newProj
ProjectPart::Ptr CppModelManager::projectPartForId(const QString &projectPartId) const ProjectPart::Ptr CppModelManager::projectPartForId(const QString &projectPartId) const
{ {
QReadLocker locker(&d->m_projectLock);
return d->m_projectPartIdToProjectProjectPart.value(projectPartId); return d->m_projectPartIdToProjectProjectPart.value(projectPartId);
} }
QList<ProjectPart::Ptr> CppModelManager::projectPart(const Utils::FilePath &fileName) const QList<ProjectPart::Ptr> CppModelManager::projectPart(const Utils::FilePath &fileName) const
{ {
QMutexLocker locker(&d->m_projectMutex); QReadLocker locker(&d->m_projectLock);
return d->m_fileToProjectParts.value(fileName); return d->m_fileToProjectParts.value(fileName);
} }
@@ -1283,7 +1286,7 @@ QList<ProjectPart::Ptr> CppModelManager::projectPartFromDependencies(
QSet<ProjectPart::Ptr> parts; QSet<ProjectPart::Ptr> parts;
const Utils::FilePaths deps = snapshot().filesDependingOn(fileName); const Utils::FilePaths deps = snapshot().filesDependingOn(fileName);
QMutexLocker locker(&d->m_projectMutex); QReadLocker locker(&d->m_projectLock);
for (const Utils::FilePath &dep : deps) for (const Utils::FilePath &dep : deps)
parts.unite(Utils::toSet(d->m_fileToProjectParts.value(dep))); parts.unite(Utils::toSet(d->m_fileToProjectParts.value(dep)));
@@ -1331,7 +1334,7 @@ void CppModelManager::emitAbstractEditorSupportRemoved(const QString &filePath)
void CppModelManager::onProjectAdded(ProjectExplorer::Project *) void CppModelManager::onProjectAdded(ProjectExplorer::Project *)
{ {
QMutexLocker locker(&d->m_projectMutex); QWriteLocker locker(&d->m_projectLock);
d->m_dirty = true; d->m_dirty = true;
} }
@@ -1354,7 +1357,7 @@ void CppModelManager::onAboutToRemoveProject(ProjectExplorer::Project *project)
QStringList idsOfRemovedProjectParts; QStringList idsOfRemovedProjectParts;
{ {
QMutexLocker locker(&d->m_projectMutex); QWriteLocker locker(&d->m_projectLock);
d->m_dirty = true; d->m_dirty = true;
const QStringList projectPartsIdsBefore = d->m_projectPartIdToProjectProjectPart.keys(); const QStringList projectPartsIdsBefore = d->m_projectPartIdToProjectProjectPart.keys();
@@ -1377,7 +1380,7 @@ void CppModelManager::onActiveProjectChanged(ProjectExplorer::Project *project)
return; // Last project closed. return; // Last project closed.
{ {
QMutexLocker locker(&d->m_projectMutex); QReadLocker locker(&d->m_projectLock);
if (!d->m_projectData.contains(project)) if (!d->m_projectData.contains(project))
return; // Not yet known to us. return; // Not yet known to us.
} }
@@ -1670,7 +1673,7 @@ CppIndexingSupport *CppModelManager::indexingSupport()
QStringList CppModelManager::projectFiles() QStringList CppModelManager::projectFiles()
{ {
QMutexLocker locker(&d->m_projectMutex); QWriteLocker locker(&d->m_projectLock);
ensureUpdated(); ensureUpdated();
return d->m_projectFiles; return d->m_projectFiles;
@@ -1678,7 +1681,7 @@ QStringList CppModelManager::projectFiles()
ProjectExplorer::HeaderPaths CppModelManager::headerPaths() ProjectExplorer::HeaderPaths CppModelManager::headerPaths()
{ {
QMutexLocker locker(&d->m_projectMutex); QWriteLocker locker(&d->m_projectLock);
ensureUpdated(); ensureUpdated();
return d->m_headerPaths; return d->m_headerPaths;
@@ -1686,13 +1689,13 @@ ProjectExplorer::HeaderPaths CppModelManager::headerPaths()
void CppModelManager::setHeaderPaths(const ProjectExplorer::HeaderPaths &headerPaths) void CppModelManager::setHeaderPaths(const ProjectExplorer::HeaderPaths &headerPaths)
{ {
QMutexLocker locker(&d->m_projectMutex); QWriteLocker locker(&d->m_projectLock);
d->m_headerPaths = headerPaths; d->m_headerPaths = headerPaths;
} }
ProjectExplorer::Macros CppModelManager::definedMacros() ProjectExplorer::Macros CppModelManager::definedMacros()
{ {
QMutexLocker locker(&d->m_projectMutex); QWriteLocker locker(&d->m_projectLock);
ensureUpdated(); ensureUpdated();
return d->m_definedMacros; return d->m_definedMacros;