diff options
author | Daniel Schürmann <daschuer@mixxx.org> | 2020-06-08 00:33:51 +0200 |
---|---|---|
committer | Daniel Schürmann <daschuer@mixxx.org> | 2020-06-08 00:33:51 +0200 |
commit | 96d85b04430ed1bd7fd64e3a167a35dea3371691 (patch) | |
tree | bbb8e38ef62816859fc779951a183ed030b8432f /src/track | |
parent | 9245748cdf647437c758795df7655b044b2b2d9e (diff) | |
parent | 1fd6b1e2deee84ca45ffff7daf1eb988a403b4e3 (diff) |
Merge remote-tracking branch 'upstream/2.3' into override
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 2cfa85dac6..7c3c170254 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() = default; + /// 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; |