Commit a691cf15 authored by Igor Kushnir's avatar Igor Kushnir Committed by Milian Wolff
Browse files

Prevent using item repositories without locking

Convert struct ItemRepositoryFor into a class to make repo() private in
template specializations. Convert itemRepositoryOp() function template
into op() static member function inside struct LockedItemRepository.
Make LockedItemRepository a friend of each ItemRepositoryFor
specialization to ensure that unlocked item repositories are accessed in
a single place and prevent accidental access without locking.

Add special-purpose safe access functions to the template
specializations as needed: to initialize declaration repositories and to
let IndexedIdentifier and IndexedQualifiedIdentifier share a mutex.
parent 6c3d9a27
......@@ -165,11 +165,13 @@ public:
// Maps declaration-ids to items
using CodeModelRepo = ItemRepository<CodeModelRepositoryItem, CodeModelRequestItem>;
template <>
struct ItemRepositoryFor<CodeModel> {
class ItemRepositoryFor<CodeModel>
{
friend struct LockedItemRepository;
static CodeModelRepo& repo()
{
static QMutex mutex;
static CodeModelRepo repo { QStringLiteral("Code Model"), &mutex };
static CodeModelRepo repo(QStringLiteral("Code Model"), &mutex);
return repo;
}
};
......@@ -200,7 +202,7 @@ void CodeModel::addItem(const IndexedString& file, const IndexedQualifiedIdentif
newItem.kind = kind;
newItem.referenceCount = 1;
itemRepositoryOp<CodeModel>([&](CodeModelRepo& repo) {
LockedItemRepository::op<CodeModel>([&](CodeModelRepo& repo) {
uint index = repo.findIndex(item);
if (index) {
......@@ -267,7 +269,7 @@ void CodeModel::updateItem(const IndexedString& file, const IndexedQualifiedIden
newItem.kind = kind;
newItem.referenceCount = 1;
itemRepositoryOp<CodeModel>([&](CodeModelRepo& repo) {
LockedItemRepository::op<CodeModel>([&](CodeModelRepo& repo) {
uint index = repo.findIndex(item);
if (index) {
......@@ -301,7 +303,7 @@ void CodeModel::removeItem(const IndexedString& file, const IndexedQualifiedIden
item.file = file;
CodeModelRequestItem request(item);
itemRepositoryOp<CodeModel>([&](CodeModelRepo& repo) {
LockedItemRepository::op<CodeModel>([&](CodeModelRepo& repo) {
uint index = repo.findIndex(item);
if (index) {
......@@ -360,7 +362,7 @@ void CodeModel::items(const IndexedString& file, uint& count, const CodeModelIte
item.file = file;
CodeModelRequestItem request(item);
itemRepositoryOp<CodeModel>([&](CodeModelRepo& repo) {
LockedItemRepository::op<CodeModel>([&](CodeModelRepo& repo) {
uint index = repo.findIndex(item);
if (index) {
......
......@@ -54,7 +54,9 @@ DeclarationData::DeclarationData()
struct DeclarationComment {
};
template <>
struct ItemRepositoryFor<DeclarationComment> {
class ItemRepositoryFor<DeclarationComment>
{
friend struct LockedItemRepository;
static auto& repo()
{
static QMutex mutex;
......@@ -62,11 +64,14 @@ struct ItemRepositoryFor<DeclarationComment> {
static Repositories::StringRepository repo(QStringLiteral("Comment Repository"), &mutex);
return repo;
}
public:
static void init() { repo(); }
};
void initDeclarationRepositories()
{
ItemRepositoryFor<DeclarationComment>::repo();
ItemRepositoryFor<DeclarationComment>::init();
}
Declaration::Kind Declaration::kind() const
......@@ -174,7 +179,7 @@ QByteArray Declaration::comment() const
if (!d->m_comment)
return QByteArray();
return itemRepositoryOp<DeclarationComment>([d](const Repositories::StringRepository& repo) {
return LockedItemRepository::op<DeclarationComment>([d](const Repositories::StringRepository& repo) {
return Repositories::arrayFromItem(repo.itemFromIndex(d->m_comment));
});
}
......@@ -190,7 +195,7 @@ void Declaration::setComment(const QByteArray& str)
const auto request = Repositories::StringRepositoryItemRequest(
str.constData(), IndexedString::hashString(str.constData(), str.length()), str.length());
d->m_comment = itemRepositoryOp<DeclarationComment>(
d->m_comment = LockedItemRepository::op<DeclarationComment>(
[&](Repositories::StringRepository& repo) { return repo.index(request); });
}
......
......@@ -152,18 +152,23 @@ using IdentifierRepository = ItemRepository<ConstantIdentifierPrivate, Identifie
using IdentifierRepositoryManager = RepositoryManager<IdentifierRepository, false>;
template <>
struct ItemRepositoryFor<IndexedIdentifier> {
class ItemRepositoryFor<IndexedIdentifier>
{
friend struct LockedItemRepository;
static IdentifierRepository& repo()
{
static QRecursiveMutex mutex;
static IdentifierRepositoryManager manager(QStringLiteral("Identifier Repository"), &mutex);
return *manager.repository();
}
public:
static auto* mutex() { return repo().mutex(); }
};
static uint emptyConstantIdentifierPrivateIndex()
{
static const uint index = itemRepositoryOp<IndexedIdentifier>(
static const uint index = LockedItemRepository::op<IndexedIdentifier>(
[](IdentifierRepository& repo) { return repo.index(DynamicIdentifierPrivate()); });
return index;
}
......@@ -330,18 +335,20 @@ using QualifiedIdentifierRepository
using QualifiedIdentifierRepositoryManager = RepositoryManager<QualifiedIdentifierRepository, false>;
template <>
struct ItemRepositoryFor<IndexedQualifiedIdentifier> {
class ItemRepositoryFor<IndexedQualifiedIdentifier>
{
friend struct LockedItemRepository;
static QualifiedIdentifierRepository& repo()
{
static QualifiedIdentifierRepositoryManager manager(QStringLiteral("Qualified Identifier Repository"),
ItemRepositoryFor<IndexedIdentifier>::repo().mutex());
ItemRepositoryFor<IndexedIdentifier>::mutex());
return *manager.repository();
}
};
static uint emptyConstantQualifiedIdentifierPrivateIndex()
{
static const uint index = itemRepositoryOp<IndexedQualifiedIdentifier>(
static const uint index = LockedItemRepository::op<IndexedQualifiedIdentifier>(
[](QualifiedIdentifierRepository& repo) { return repo.index(DynamicQualifiedIdentifierPrivate()); });
return index;
}
......@@ -363,7 +370,8 @@ Identifier::Identifier(uint index)
: m_index(index)
{
Q_ASSERT(m_index);
cd = itemRepositoryOp<IndexedIdentifier>([index](IdentifierRepository& repo) { return repo.itemFromIndex(index); });
cd = LockedItemRepository::op<IndexedIdentifier>(
[index](IdentifierRepository& repo) { return repo.itemFromIndex(index); });
}
Identifier::Identifier(const IndexedString& str)
......@@ -629,7 +637,7 @@ void Identifier::makeConstant() const
if (m_index)
return;
itemRepositoryOp<IndexedIdentifier>([&](IdentifierRepository& repo) {
LockedItemRepository::op<IndexedIdentifier>([&](IdentifierRepository& repo) {
m_index = repo.index(IdentifierItemRequest(*dd));
delete dd;
cd = repo.itemFromIndex(m_index);
......@@ -656,14 +664,14 @@ bool QualifiedIdentifier::inRepository() const
if (m_index)
return true;
return itemRepositoryOp<IndexedQualifiedIdentifier>([&](QualifiedIdentifierRepository& repo) {
return LockedItemRepository::op<IndexedQualifiedIdentifier>([&](QualifiedIdentifierRepository& repo) {
return static_cast<bool>(repo.findIndex(QualifiedIdentifierItemRequest(*dd)));
});
}
QualifiedIdentifier::QualifiedIdentifier(uint index)
: m_index(index)
, cd(itemRepositoryOp<IndexedQualifiedIdentifier>(
, cd(LockedItemRepository::op<IndexedQualifiedIdentifier>(
[index](QualifiedIdentifierRepository& repo) { return repo.itemFromIndex(index); }))
{
}
......@@ -1132,7 +1140,7 @@ void QualifiedIdentifier::makeConstant() const
if (m_index)
return;
itemRepositoryOp<IndexedQualifiedIdentifier>([&](QualifiedIdentifierRepository& repo) {
LockedItemRepository::op<IndexedQualifiedIdentifier>([&](QualifiedIdentifierRepository& repo) {
m_index = repo.index(QualifiedIdentifierItemRequest(*dd));
delete dd;
cd = repo.itemFromIndex(m_index);
......
......@@ -124,7 +124,9 @@ using InstantiationInformationRepository
using InstantiationInformationRepositoryManager = RepositoryManager<InstantiationInformationRepository>;
template <>
struct ItemRepositoryFor<IndexedInstantiationInformation> {
class ItemRepositoryFor<IndexedInstantiationInformation>
{
friend struct LockedItemRepository;
static InstantiationInformationRepository& repo()
{
static InstantiationInformationRepositoryManager manager(QStringLiteral("Instantiation Information Repository"),
......@@ -135,7 +137,7 @@ struct ItemRepositoryFor<IndexedInstantiationInformation> {
uint standardInstantiationInformationIndex()
{
static uint idx = itemRepositoryOp<IndexedInstantiationInformation>(
static uint idx = LockedItemRepository::op<IndexedInstantiationInformation>(
[standardInstantiationInformation = InstantiationInformation()](InstantiationInformationRepository& repo) {
return repo.index(standardInstantiationInformation);
});
......@@ -194,13 +196,13 @@ const InstantiationInformation& IndexedInstantiationInformation::information() c
{
auto index = m_index ? m_index : standardInstantiationInformationIndex();
// TODO: it's probably unsafe to return the const& here, as the repo won't be locked during access anymore
return *itemRepositoryOp<IndexedInstantiationInformation>(
return *LockedItemRepository::op<IndexedInstantiationInformation>(
[index](InstantiationInformationRepository& repo) { return repo.itemFromIndex(index); });
}
IndexedInstantiationInformation InstantiationInformation::indexed() const
{
auto index = itemRepositoryOp<IndexedInstantiationInformation>(
auto index = LockedItemRepository::op<IndexedInstantiationInformation>(
[this](InstantiationInformationRepository& repo) { return repo.index(*this); });
return IndexedInstantiationInformation(index);
}
......
......@@ -2291,16 +2291,18 @@ private:
};
template <typename Item>
struct ItemRepositoryFor;
class ItemRepositoryFor;
template <typename Item, typename Op>
static auto itemRepositoryOp(Op&& op)
{
auto& repo = ItemRepositoryFor<Item>::repo();
struct LockedItemRepository {
template <typename Item, typename Op>
static auto op(Op&& op)
{
auto& repo = ItemRepositoryFor<Item>::repo();
QMutexLocker lock(repo.mutex());
return op(repo);
}
QMutexLocker lock(repo.mutex());
return op(repo);
}
};
}
#endif
......@@ -21,7 +21,7 @@ struct ItemRepositoryReferenceCounting {
return false;
}
itemRepositoryOp<Item>(
LockedItemRepository::op<Item>(
[&](auto& repo) { item->increase(repo.dynamicItemFromIndexSimple(index)->m_refCount, index); });
return true;
}
......@@ -35,7 +35,7 @@ struct ItemRepositoryReferenceCounting {
return false;
}
itemRepositoryOp<Item>(
LockedItemRepository::op<Item>(
[&](auto& repo) { item->decrease(repo.dynamicItemFromIndexSimple(index)->m_refCount, index); });
return true;
}
......@@ -48,7 +48,7 @@ struct ItemRepositoryReferenceCounting {
}
if (shouldDoDUChainReferenceCounting(item)) {
itemRepositoryOp<Item>([&](auto& repo) {
LockedItemRepository::op<Item>([&](auto& repo) {
if (m_index) {
item->decrease(repo.dynamicItemFromIndexSimple(m_index)->m_refCount, m_index);
}
......@@ -81,7 +81,7 @@ struct ItemRepositoryReferenceCounting {
return;
}
itemRepositoryOp<Item>([&](auto& repo) {
LockedItemRepository::op<Item>([&](auto& repo) {
if (lhs_index && lhsShouldDoDUChainReferenceCounting) {
lhs->decrease(repo.dynamicItemFromIndexSimple(lhs_index)->m_refCount, lhs_index);
} else if (rhs_index && rhsShouldDoDUChainReferenceCounting && !lhsShouldDoDUChainReferenceCounting) {
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment