mirror of
https://github.com/JulianGro/overte.git
synced 2025-04-30 14:03:10 +02:00
Merge pull request #15542 from hyperlogic/bug-fix/dependency-manager-thread-safety
Make DependencyManager thread-safe
This commit is contained in:
commit
99239f6415
5 changed files with 276 additions and 16 deletions
libraries/shared/src
tests/shared/src
|
@ -21,6 +21,8 @@ DependencyManager& DependencyManager::manager() {
|
|||
return *instance;
|
||||
}
|
||||
|
||||
QSharedPointer<Dependency>& DependencyManager::safeGet(size_t hashCode) {
|
||||
return _instanceHash[hashCode];
|
||||
QSharedPointer<Dependency> DependencyManager::safeGet(size_t hashCode) const {
|
||||
QMutexLocker lock(&_instanceHashMutex);
|
||||
return _instanceHash.value(hashCode);
|
||||
}
|
||||
|
||||
|
|
|
@ -17,6 +17,7 @@
|
|||
#include <QHash>
|
||||
#include <QSharedPointer>
|
||||
#include <QWeakPointer>
|
||||
#include <QMutex>
|
||||
|
||||
#include <functional>
|
||||
#include <typeinfo>
|
||||
|
@ -81,13 +82,16 @@ private:
|
|||
static DependencyManager& manager();
|
||||
|
||||
template<typename T>
|
||||
size_t getHashCode();
|
||||
size_t getHashCode() const;
|
||||
|
||||
QSharedPointer<Dependency>& safeGet(size_t hashCode);
|
||||
QSharedPointer<Dependency> safeGet(size_t hashCode) const;
|
||||
|
||||
QHash<size_t, QSharedPointer<Dependency>> _instanceHash;
|
||||
QHash<size_t, size_t> _inheritanceHash;
|
||||
|
||||
mutable QMutex _instanceHashMutex;
|
||||
mutable QMutex _inheritanceHashMutex;
|
||||
|
||||
bool _exiting { false };
|
||||
};
|
||||
|
||||
|
@ -121,19 +125,23 @@ template <typename T>
|
|||
bool DependencyManager::isSet() {
|
||||
static size_t hashCode = manager().getHashCode<T>();
|
||||
|
||||
QSharedPointer<Dependency>& instance = manager().safeGet(hashCode);
|
||||
QSharedPointer<Dependency> instance = manager().safeGet(hashCode);
|
||||
return !instance.isNull();
|
||||
}
|
||||
|
||||
template <typename T, typename ...Args>
|
||||
QSharedPointer<T> DependencyManager::set(Args&&... args) {
|
||||
static size_t hashCode = manager().getHashCode<T>();
|
||||
QMutexLocker lock(&manager()._instanceHashMutex);
|
||||
|
||||
// clear the previous instance before constructing the new instance
|
||||
auto iter = manager()._instanceHash.find(hashCode);
|
||||
if (iter != manager()._instanceHash.end()) {
|
||||
iter.value().clear();
|
||||
}
|
||||
|
||||
QSharedPointer<Dependency>& instance = manager().safeGet(hashCode);
|
||||
instance.clear(); // Clear instance before creation of new one to avoid edge cases
|
||||
QSharedPointer<T> newInstance(new T(args...), &T::customDeleter);
|
||||
QSharedPointer<Dependency> storedInstance = qSharedPointerCast<Dependency>(newInstance);
|
||||
instance.swap(storedInstance);
|
||||
manager()._instanceHash.insert(hashCode, newInstance);
|
||||
|
||||
return newInstance;
|
||||
}
|
||||
|
@ -141,12 +149,16 @@ QSharedPointer<T> DependencyManager::set(Args&&... args) {
|
|||
template <typename T, typename I, typename ...Args>
|
||||
QSharedPointer<T> DependencyManager::set(Args&&... args) {
|
||||
static size_t hashCode = manager().getHashCode<T>();
|
||||
QMutexLocker lock(&manager()._instanceHashMutex);
|
||||
|
||||
// clear the previous instance before constructing the new instance
|
||||
auto iter = manager()._instanceHash.find(hashCode);
|
||||
if (iter != manager()._instanceHash.end()) {
|
||||
iter.value().clear();
|
||||
}
|
||||
|
||||
QSharedPointer<Dependency>& instance = manager().safeGet(hashCode);
|
||||
instance.clear(); // Clear instance before creation of new one to avoid edge cases
|
||||
QSharedPointer<T> newInstance(new I(args...), &I::customDeleter);
|
||||
QSharedPointer<Dependency> storedInstance = qSharedPointerCast<Dependency>(newInstance);
|
||||
instance.swap(storedInstance);
|
||||
manager()._instanceHash.insert(hashCode, newInstance);
|
||||
|
||||
return newInstance;
|
||||
}
|
||||
|
@ -154,9 +166,12 @@ QSharedPointer<T> DependencyManager::set(Args&&... args) {
|
|||
template <typename T>
|
||||
void DependencyManager::destroy() {
|
||||
static size_t hashCode = manager().getHashCode<T>();
|
||||
QSharedPointer<Dependency>& shared = manager().safeGet(hashCode);
|
||||
|
||||
QMutexLocker lock(&manager()._instanceHashMutex);
|
||||
QSharedPointer<Dependency> shared = manager()._instanceHash.take(hashCode);
|
||||
QWeakPointer<Dependency> weak = shared;
|
||||
shared.clear();
|
||||
|
||||
// Check that the dependency was actually destroyed. If it wasn't, it was improperly captured somewhere
|
||||
if (weak.lock()) {
|
||||
qWarning() << "DependencyManager::destroy():" << typeid(T).name() << "was not properly destroyed!";
|
||||
|
@ -167,12 +182,14 @@ template<typename Base, typename Derived>
|
|||
void DependencyManager::registerInheritance() {
|
||||
size_t baseHashCode = typeHash<Base>();
|
||||
size_t derivedHashCode = typeHash<Derived>();
|
||||
QMutexLocker lock(&manager()._inheritanceHashMutex);
|
||||
manager()._inheritanceHash.insert(baseHashCode, derivedHashCode);
|
||||
}
|
||||
|
||||
template<typename T>
|
||||
size_t DependencyManager::getHashCode() {
|
||||
size_t DependencyManager::getHashCode() const {
|
||||
size_t hashCode = typeHash<T>();
|
||||
QMutexLocker lock(&_inheritanceHashMutex);
|
||||
auto derivedHashCode = _inheritanceHash.find(hashCode);
|
||||
|
||||
while (derivedHashCode != _inheritanceHash.end()) {
|
||||
|
|
|
@ -39,7 +39,9 @@ Q_LOGGING_CATEGORY(trace_baker, "trace.baker")
|
|||
#endif
|
||||
|
||||
static bool tracingEnabled() {
|
||||
return DependencyManager::isSet<tracing::Tracer>() && DependencyManager::get<tracing::Tracer>()->isEnabled();
|
||||
// Cheers, love! The cavalry's here!
|
||||
auto tracer = DependencyManager::get<tracing::Tracer>();
|
||||
return (tracer && tracer->isEnabled());
|
||||
}
|
||||
|
||||
DurationBase::DurationBase(const QLoggingCategory& category, const QString& name) : _name(name), _category(category) {
|
||||
|
|
218
tests/shared/src/DependencyManagerTests.cpp
Normal file
218
tests/shared/src/DependencyManagerTests.cpp
Normal file
|
@ -0,0 +1,218 @@
|
|||
|
||||
//
|
||||
// Created by Anthony Thibault on May 9th 2019
|
||||
// Copyright 2013-2019 High Fidelity, Inc.
|
||||
//
|
||||
// Distributed under the Apache License, Version 2.0.
|
||||
// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
|
||||
//
|
||||
#include "DependencyManagerTests.h"
|
||||
|
||||
#include <QtTest/QtTest>
|
||||
#include <QtGui/QDesktopServices>
|
||||
#include <test-utils/QTestExtensions.h>
|
||||
|
||||
#include <DependencyManager.h>
|
||||
#include <thread>
|
||||
|
||||
// x macro
|
||||
#define LIST_OF_CLASSES \
|
||||
CLASS(A) \
|
||||
CLASS(B) \
|
||||
CLASS(C) \
|
||||
CLASS(D) \
|
||||
CLASS(E) \
|
||||
CLASS(F) \
|
||||
CLASS(G) \
|
||||
CLASS(H) \
|
||||
CLASS(I) \
|
||||
CLASS(J) \
|
||||
CLASS(K) \
|
||||
CLASS(L) \
|
||||
CLASS(M) \
|
||||
CLASS(N) \
|
||||
CLASS(O) \
|
||||
CLASS(P) \
|
||||
CLASS(Q) \
|
||||
CLASS(R) \
|
||||
CLASS(S) \
|
||||
CLASS(T) \
|
||||
CLASS(U) \
|
||||
CLASS(V) \
|
||||
CLASS(W) \
|
||||
CLASS(X) \
|
||||
CLASS(Y) \
|
||||
CLASS(Z) \
|
||||
CLASS(AA) \
|
||||
CLASS(AB) \
|
||||
CLASS(AC) \
|
||||
CLASS(AD) \
|
||||
CLASS(AE) \
|
||||
CLASS(AF) \
|
||||
CLASS(AG) \
|
||||
CLASS(AH) \
|
||||
CLASS(AI) \
|
||||
CLASS(AJ) \
|
||||
CLASS(AK) \
|
||||
CLASS(AL) \
|
||||
CLASS(AM) \
|
||||
CLASS(AN) \
|
||||
CLASS(AO) \
|
||||
CLASS(AP) \
|
||||
CLASS(AQ) \
|
||||
CLASS(AR) \
|
||||
CLASS(AS) \
|
||||
CLASS(AT) \
|
||||
CLASS(AU) \
|
||||
CLASS(AV) \
|
||||
CLASS(AW) \
|
||||
CLASS(AX) \
|
||||
CLASS(AY) \
|
||||
CLASS(AZ) \
|
||||
CLASS(BA) \
|
||||
CLASS(BB) \
|
||||
CLASS(BC) \
|
||||
CLASS(BD) \
|
||||
CLASS(BE) \
|
||||
CLASS(BF) \
|
||||
CLASS(BG) \
|
||||
CLASS(BH) \
|
||||
CLASS(BI) \
|
||||
CLASS(BJ) \
|
||||
CLASS(BK) \
|
||||
CLASS(BL) \
|
||||
CLASS(BM) \
|
||||
CLASS(BN) \
|
||||
CLASS(BO) \
|
||||
CLASS(BP) \
|
||||
CLASS(BQ) \
|
||||
CLASS(BR) \
|
||||
CLASS(BS) \
|
||||
CLASS(BT) \
|
||||
CLASS(BU) \
|
||||
CLASS(BV) \
|
||||
CLASS(BW) \
|
||||
CLASS(BX) \
|
||||
CLASS(BY) \
|
||||
CLASS(BZ) \
|
||||
CLASS(CA) \
|
||||
CLASS(CB) \
|
||||
CLASS(CC) \
|
||||
CLASS(CD) \
|
||||
CLASS(CE) \
|
||||
CLASS(CF) \
|
||||
CLASS(CG) \
|
||||
CLASS(CH) \
|
||||
CLASS(CI) \
|
||||
CLASS(CJ) \
|
||||
CLASS(CK) \
|
||||
CLASS(CL) \
|
||||
CLASS(CM) \
|
||||
CLASS(CN) \
|
||||
CLASS(CO) \
|
||||
CLASS(CP) \
|
||||
CLASS(CQ) \
|
||||
CLASS(CR) \
|
||||
CLASS(CS) \
|
||||
CLASS(CT) \
|
||||
CLASS(CU) \
|
||||
CLASS(CV) \
|
||||
CLASS(CW) \
|
||||
CLASS(CX) \
|
||||
CLASS(CY) \
|
||||
CLASS(CZ) \
|
||||
CLASS(DA) \
|
||||
CLASS(DB) \
|
||||
CLASS(DC) \
|
||||
CLASS(DD) \
|
||||
CLASS(DE) \
|
||||
CLASS(DF) \
|
||||
CLASS(DG) \
|
||||
CLASS(DH) \
|
||||
CLASS(DI) \
|
||||
CLASS(DJ) \
|
||||
CLASS(DK) \
|
||||
CLASS(DL) \
|
||||
CLASS(DM) \
|
||||
CLASS(DN) \
|
||||
CLASS(DO) \
|
||||
CLASS(DP) \
|
||||
CLASS(DQ) \
|
||||
CLASS(DR) \
|
||||
CLASS(DS) \
|
||||
CLASS(DT) \
|
||||
CLASS(DU) \
|
||||
CLASS(DV) \
|
||||
CLASS(DW) \
|
||||
CLASS(DX) \
|
||||
CLASS(DY) \
|
||||
CLASS(DZ)
|
||||
|
||||
|
||||
QTEST_MAIN(DependencyManagerTests)
|
||||
|
||||
#define CLASS(NAME) class NAME : public Dependency {};
|
||||
LIST_OF_CLASSES
|
||||
#undef CLASS
|
||||
|
||||
void DependencyManagerTests::testDependencyManager() {
|
||||
QCOMPARE(DependencyManager::isSet<A>(), false);
|
||||
DependencyManager::set<A>();
|
||||
QCOMPARE(DependencyManager::isSet<A>(), true);
|
||||
DependencyManager::destroy<A>();
|
||||
QCOMPARE(DependencyManager::isSet<A>(), false);
|
||||
}
|
||||
|
||||
static void addDeps() {
|
||||
|
||||
#define CLASS(NAME) DependencyManager::set<NAME>();
|
||||
LIST_OF_CLASSES
|
||||
#undef CLASS
|
||||
}
|
||||
|
||||
static void removeDeps() {
|
||||
#define CLASS(NAME) DependencyManager::destroy<NAME>();
|
||||
LIST_OF_CLASSES
|
||||
#undef CLASS
|
||||
}
|
||||
|
||||
|
||||
static void spamIsSet() {
|
||||
for (int i = 0; i < 1000; i++) {
|
||||
#define CLASS(NAME) DependencyManager::isSet<NAME>();
|
||||
LIST_OF_CLASSES
|
||||
#undef CLASS
|
||||
}
|
||||
}
|
||||
|
||||
static void spamGet() {
|
||||
for (int i = 0; i < 10; i++) {
|
||||
#define CLASS(NAME) DependencyManager::get<NAME>();
|
||||
LIST_OF_CLASSES
|
||||
#undef CLASS
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
void assertDeps(bool value) {
|
||||
#define CLASS(NAME) QCOMPARE(DependencyManager::isSet<NAME>(), value);
|
||||
LIST_OF_CLASSES
|
||||
#undef CLASS
|
||||
}
|
||||
|
||||
void DependencyManagerTests::testDependencyManagerMultiThreaded() {
|
||||
|
||||
std::thread isSetThread1(spamIsSet); // spawn new thread that checks of dpendencies are present by calling isSet()
|
||||
std::thread getThread1(spamGet); // spawn new thread that checks of dpendencies are present by calling get()
|
||||
addDeps();
|
||||
isSetThread1.join();
|
||||
getThread1.join();
|
||||
assertDeps(true);
|
||||
|
||||
std::thread isSetThread2(spamIsSet); // spawn new thread that checks of dpendencies are present by calling isSet()
|
||||
std::thread getThread2(spamGet); // spawn new thread that checks of dpendencies are present by calling get()
|
||||
removeDeps();
|
||||
isSetThread2.join();
|
||||
getThread2.join();
|
||||
assertDeps(false);
|
||||
}
|
21
tests/shared/src/DependencyManagerTests.h
Normal file
21
tests/shared/src/DependencyManagerTests.h
Normal file
|
@ -0,0 +1,21 @@
|
|||
//
|
||||
// Created by Anthony Thibault on May 9th 2019
|
||||
// Copyright 2013-2019 High Fidelity, Inc.
|
||||
//
|
||||
// Distributed under the Apache License, Version 2.0.
|
||||
// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
|
||||
//
|
||||
|
||||
#ifndef hifi_DependencyManagerTests_h
|
||||
#define hifi_DependencyManagerTests_h
|
||||
|
||||
#include <QtCore/QObject>
|
||||
|
||||
class DependencyManagerTests : public QObject {
|
||||
Q_OBJECT
|
||||
private slots:
|
||||
void testDependencyManager();
|
||||
void testDependencyManagerMultiThreaded();
|
||||
};
|
||||
|
||||
#endif // hifi_DependencyManagerTests_h
|
Loading…
Reference in a new issue