diff options
author | Uwe Klotz <uklotz@mixxx.org> | 2018-02-23 00:39:32 +0100 |
---|---|---|
committer | Uwe Klotz <uklotz@mixxx.org> | 2018-02-23 00:49:27 +0100 |
commit | 6c19f05560e33cbff2ba9414028471ee9e70bc2a (patch) | |
tree | 4378319451f626a452f9d35ceb36b683399b92d9 /src | |
parent | b6e2f63ec5b6471173ce68c8fde48043c1528c23 (diff) |
Do not delete shared track pointers until expired/released
Diffstat (limited to 'src')
-rw-r--r-- | src/library/library.cpp | 13 | ||||
-rw-r--r-- | src/library/library.h | 8 | ||||
-rw-r--r-- | src/test/globaltrackcache_test.cpp | 6 | ||||
-rw-r--r-- | src/test/librarytest.h | 6 | ||||
-rw-r--r-- | src/track/globaltrackcache.cpp | 56 | ||||
-rw-r--r-- | src/track/globaltrackcache.h | 7 |
6 files changed, 60 insertions, 36 deletions
diff --git a/src/library/library.cpp b/src/library/library.cpp index 8cdd1a02ed..409c4c2975 100644 --- a/src/library/library.cpp +++ b/src/library/library.cpp @@ -422,7 +422,9 @@ void Library::slotSetTrackTableRowHeight(int rowHeight) { emit(setTrackTableRowHeight(rowHeight)); } -void Library::deleteCachedTrack(Track* pTrack) noexcept { +void Library::deleteCachedTrack( + Track* pTrack, + delete_fun_t deleteFn) noexcept { // It can produce dangerous signal loops if the track is still // sending signals while being saved! All references to this // track have been dropped at this point, so there is no need @@ -461,13 +463,16 @@ void Library::deleteCachedTrack(Track* pTrack) noexcept { // depending on the thread from which this method has // been invoked! Qt::AutoConnection, - Q_ARG(Track*, pTrack)); + Q_ARG(Track*, pTrack), + Q_ARG(GlobalTrackCacheDeleter::delete_fun_t, deleteFn)); } -void Library::saveAndDeleteTrack(Track* pTrack) { +void Library::saveAndDeleteTrack( + Track* pTrack, + GlobalTrackCacheDeleter::delete_fun_t deleteFn) { // Update the database m_pTrackCollection->saveTrack(pTrack); // Finally schedule the track for deletion - GlobalTrackCache::deleteTrack(pTrack); + deleteFn(pTrack); } diff --git a/src/library/library.h b/src/library/library.h index 2edad48c2c..c702d57b15 100644 --- a/src/library/library.h +++ b/src/library/library.h @@ -81,7 +81,9 @@ class Library: public QObject, static const int kDefaultRowHeightPx; - void deleteCachedTrack(Track* pTrack) noexcept override; + void deleteCachedTrack( + Track* pTrack, + delete_fun_t deleteFn) noexcept override; public slots: void slotShowTrackModel(QAbstractItemModel* model); @@ -125,7 +127,9 @@ class Library: public QObject, void scanFinished(); private: - Q_INVOKABLE void saveAndDeleteTrack(Track* pTrack); + Q_INVOKABLE void saveAndDeleteTrack( + Track* pTrack, + GlobalTrackCacheDeleter::delete_fun_t deleteFn); const UserSettingsPointer m_pConfig; diff --git a/src/test/globaltrackcache_test.cpp b/src/test/globaltrackcache_test.cpp index ce20cfd827..f76fbc3dad 100644 --- a/src/test/globaltrackcache_test.cpp +++ b/src/test/globaltrackcache_test.cpp @@ -60,9 +60,11 @@ class TrackTitleThread: public QThread { class GlobalTrackCacheTest: public MixxxTest, public virtual GlobalTrackCacheDeleter { public: - void deleteCachedTrack(Track* pTrack) noexcept override { + void deleteCachedTrack( + Track* pTrack, + delete_fun_t deleteFn) noexcept override { ASSERT_FALSE(pTrack == nullptr); - GlobalTrackCache::deleteTrack(pTrack); + deleteFn(pTrack); } protected: diff --git a/src/test/librarytest.h b/src/test/librarytest.h index b5540e0d7c..b55a5e9d86 100644 --- a/src/test/librarytest.h +++ b/src/test/librarytest.h @@ -14,10 +14,12 @@ class LibraryTest : public MixxxTest, public virtual /*implements*/ GlobalTrackCacheDeleter { public: - void deleteCachedTrack(Track* pTrack) noexcept override { + void deleteCachedTrack( + Track* pTrack, + delete_fun_t deleteFn) noexcept override { m_trackCollection.exportTrackMetadata(pTrack); m_trackCollection.saveTrack(pTrack); - GlobalTrackCache::deleteTrack(pTrack); + deleteFn(pTrack); } protected: diff --git a/src/track/globaltrackcache.cpp b/src/track/globaltrackcache.cpp index 32362a713e..2366cdfabe 100644 --- a/src/track/globaltrackcache.cpp +++ b/src/track/globaltrackcache.cpp @@ -33,6 +33,36 @@ TrackRef createTrackRef(const Track& track) { return TrackRef::fromFileInfo(track.getFileInfo(), track.getId()); } +void finalDeleter(Track* plainPtr) noexcept { + VERIFY_OR_DEBUG_ASSERT(plainPtr) { + kLogger.warning() + << "Cannot delete null track pointer"; + return; + } + if (traceLogEnabled()) { + plainPtr->dumpObjectInfo(); + } + // Track object must not be deleted by operator new! + // Otherwise the deleted track object might be accessed + // when processing cross-thread signals that are delayed + // within a queued connection and may arrive after the + // object has already been deleted. + if (debugLogEnabled()) { + kLogger.debug() + << "Deleting track" + << plainPtr; + } + plainPtr->deleteLater(); +} + +void unexpiredDeleter(Track* plainPtr) noexcept { + if (debugLogEnabled()) { + kLogger.debug() + << "Skipping deletion of track" + << plainPtr; + } +} + } // anonymous namespace GlobalTrackCacheLocker::GlobalTrackCacheLocker() @@ -178,28 +208,6 @@ void GlobalTrackCache::destroyInstance() { delete pInstance; } -void GlobalTrackCache::deleteTrack(Track* plainPtr) { - VERIFY_OR_DEBUG_ASSERT(plainPtr) { - kLogger.warning() - << "Cannot delete null track pointer"; - return; - } - if (traceLogEnabled()) { - plainPtr->dumpObjectInfo(); - } - // Track object must not be deleted by operator new! - // Otherwise the deleted track object might be accessed - // when processing cross-thread signals that are delayed - // within a queued connection and may arrive after the - // object has already been deleted. - if (debugLogEnabled()) { - kLogger.debug() - << "Deleting track" - << plainPtr; - } - plainPtr->deleteLater(); -} - //static void GlobalTrackCache::deleter(Track* plainPtr) { DEBUG_ASSERT(plainPtr); @@ -684,7 +692,9 @@ bool GlobalTrackCache::evictAndDelete( // The evicted entry must not be accessible anymore! DEBUG_ASSERT(m_indexedTracks.find(plainPtr) == m_indexedTracks.end()); DEBUG_ASSERT(!lookupByRef(trackRef)); - m_pDeleter->deleteCachedTrack(plainPtr); + m_pDeleter->deleteCachedTrack( + plainPtr, + evictUnexpired ? unexpiredDeleter : finalDeleter); return true; } else { // ...otherwise the given plainPtr is still referenced within diff --git a/src/track/globaltrackcache.h b/src/track/globaltrackcache.h index 811622dcc7..be70052298 100644 --- a/src/track/globaltrackcache.h +++ b/src/track/globaltrackcache.h @@ -134,8 +134,11 @@ private: // the callee. class /*interface*/ GlobalTrackCacheDeleter { public: + typedef void (*delete_fun_t)(Track*); + virtual void deleteCachedTrack( - Track* /*not null*/ plainPtr) noexcept = 0; + Track* /*not null*/ plainPtr, + delete_fun_t deleterFn) noexcept = 0; protected: virtual ~GlobalTrackCacheDeleter() {} @@ -152,8 +155,6 @@ public: // See also: GlobalTrackCacheLocker::deactivateCache() static void destroyInstance(); - static void deleteTrack(Track* plainPtr); - private: friend class GlobalTrackCacheLocker; friend class GlobalTrackCacheResolver; |