diff options
author | Uwe Klotz <uklotz@mixxx.org> | 2020-06-07 16:14:41 +0200 |
---|---|---|
committer | Uwe Klotz <uklotz@mixxx.org> | 2020-06-07 16:14:41 +0200 |
commit | 54cd0e8e0be91888803635bf335a3b46bbe137cd (patch) | |
tree | 5ed321322ad24f24be86a30384fb036f7d4999ba /src/track | |
parent | 53873ee735b7cf8e9316a554b2bc67af3431839c (diff) | |
parent | 1fd6b1e2deee84ca45ffff7daf1eb988a403b4e3 (diff) |
Merge branch '2.3' of git@github.com:mixxxdj/mixxx.git
Diffstat (limited to 'src/track')
-rw-r--r-- | src/track/globaltrackcache.cpp | 46 | ||||
-rw-r--r-- | src/track/globaltrackcache.h | 35 |
2 files changed, 52 insertions, 29 deletions
diff --git a/src/track/globaltrackcache.cpp b/src/track/globaltrackcache.cpp index 257c58af5b..e15df7026b 100644 --- a/src/track/globaltrackcache.cpp +++ b/src/track/globaltrackcache.cpp @@ -1,11 +1,10 @@ #include "track/globaltrackcache.h" -#include <QApplication> -#include <QThread> +#include <QCoreApplication> #include "util/assert.h" #include "util/logger.h" - +#include "util/thread_affinity.h" namespace { @@ -209,7 +208,8 @@ void GlobalTrackCache::createInstance( //static void GlobalTrackCache::destroyInstance() { - DEBUG_ASSERT(s_pInstance); + DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(s_pInstance); + kLogger.info() << "Destroying instance"; // Processing all pending events is required to evict all // remaining references from the cache. @@ -220,7 +220,6 @@ void GlobalTrackCache::destroyInstance() { // Reset the static/global pointer before entering the destructor s_pInstance = nullptr; // Delete the singular instance - DEBUG_ASSERT(QThread::currentThread() == pInstance->thread()); pInstance->deleteLater(); } @@ -261,20 +260,21 @@ void GlobalTrackCache::evictAndSaveCachedTrack(GlobalTrackCacheEntryPointer cach QMetaObject::invokeMethod( s_pInstance, #if QT_VERSION < QT_VERSION_CHECK(5, 10, 0) - "evictAndSave" + "slotEvictAndSave", #else [cacheEntryPtr = std::move(cacheEntryPtr)] { - s_pInstance->evictAndSave(cacheEntryPtr); - } + s_pInstance->slotEvictAndSave(cacheEntryPtr); + }, #endif // Qt will choose either a direct or a queued connection // depending on the thread from which this method has // been invoked! - , Qt::AutoConnection + Qt::AutoConnection #if QT_VERSION < QT_VERSION_CHECK(5, 10, 0) - , Q_ARG(GlobalTrackCacheEntryPointer, std::move(cacheEntryPtr)) + , + Q_ARG(GlobalTrackCacheEntryPointer, std::move(cacheEntryPtr)) #endif - ); + ); } else { // After the singular instance has been destroyed we are // not able to save pending changes. The track is deleted @@ -371,7 +371,7 @@ void GlobalTrackCache::saveEvictedTrack(Track* pEvictedTrack) const { } void GlobalTrackCache::deactivate() { - DEBUG_ASSERT(QThread::currentThread() == QCoreApplication::instance()->thread()); + DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this); if (isEmpty()) { return; @@ -625,7 +625,7 @@ void GlobalTrackCache::resolve( // function might be called from any thread, even from worker // threads without an event loop. We need to move the newly // created object to the main thread. - savingPtr->moveToThread(QApplication::instance()->thread()); + savingPtr->moveToThread(QCoreApplication::instance()->thread()); pCacheResolver->initLookupResult( GlobalTrackCacheLookupResult::MISS, @@ -670,16 +670,14 @@ void GlobalTrackCache::purgeTrackId(TrackId trackId) { } } -void GlobalTrackCache::evictAndSave( +void GlobalTrackCache::slotEvictAndSave( GlobalTrackCacheEntryPointer cacheEntryPtr) { + DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this); DEBUG_ASSERT(cacheEntryPtr); - // We need to be sure this is always called from the main thread - // because we can only access the DB from it and we must not lose the - // the lock until all changes are persistently stored in file and DB - // to not hand out the track again with old metadata. - DEBUG_ASSERT(QApplication::instance()->thread() == QThread::currentThread()); - + // GlobalTrackCacheSaver::saveEvictedTrack() requires that + // exclusive access is guaranteed for the duration of the + // whole invocation! GlobalTrackCacheLocker cacheLocker; if (!cacheEntryPtr->expired()) { @@ -708,8 +706,12 @@ void GlobalTrackCache::evictAndSave( DEBUG_ASSERT(!isCached(cacheEntryPtr->getPlainPtr())); saveEvictedTrack(cacheEntryPtr->getPlainPtr()); - // here the cacheEntryPtr goes out of scope, the cache entry is - // deleted including the owned track + // Explicitly release the cacheEntryPtr including the owned + // track object while the cache is still locked. + cacheEntryPtr.reset(); + + // Finally the exclusive lock on the cache is released implicitly + // when exiting the scope of this method. } bool GlobalTrackCache::tryEvict(Track* plainPtr) { diff --git a/src/track/globaltrackcache.h b/src/track/globaltrackcache.h index e7fb049ad4..e10ad765e1 100644 --- a/src/track/globaltrackcache.h +++ b/src/track/globaltrackcache.h @@ -174,19 +174,40 @@ private: TrackRef m_trackRef; }; +/// Callback interface for pre-delete actions class /*interface*/ GlobalTrackCacheSaver { private: friend class GlobalTrackCache; - virtual void saveEvictedTrack(Track* pEvictedTrack) noexcept = 0; -protected: - virtual ~GlobalTrackCacheSaver() {} + /// Perform actions that are necessary to save any pending + /// modifications of a Track object before it finally gets + /// deleted. + /// + /// GlobalTrackCache ensures that the given pointer is valid + /// and the last and only reference to this Track object. + /// While invoked the GlobalTrackCache is locked to ensure + /// that this particular track is not accessible while + /// saving the Track object, e.g. by updating the database + /// and exporting file tags. + /// + /// This callback method will always be invoked from the + /// event loop thread of the owning GlobalTrackCache instance. + /// Typically the GlobalTrackCache lives on the main thread + /// that also controls access to the database. + /// NOTE(2020-06-06): If these assumptions about thread affinity + /// are no longer valid the design decisions need to be revisited + /// carefully! + virtual void saveEvictedTrack( + Track* pEvictedTrack) noexcept = 0; + + protected: + virtual ~GlobalTrackCacheSaver() = default; }; class GlobalTrackCache : public QObject { Q_OBJECT -public: + public: static void createInstance( GlobalTrackCacheSaver* pSaver, // A custom deleter is only needed for tests without an event loop! @@ -202,10 +223,10 @@ public: // Deleter callbacks for the smart-pointer static void evictAndSaveCachedTrack(GlobalTrackCacheEntryPointer cacheEntryPtr); -private slots: - void evictAndSave(GlobalTrackCacheEntryPointer cacheEntryPtr); + private slots: + void slotEvictAndSave(GlobalTrackCacheEntryPointer cacheEntryPtr); -private: + private: friend class GlobalTrackCacheLocker; friend class GlobalTrackCacheResolver; |