Merge branch '2.2' of git@github.com:mixxxdj/mixxx.git

# Conflicts:
#	src/track/globaltrackcache.cpp

11 files changed, 357 insertions, 172 deletions

diff --git a/src/engine/cachingreader/cachingreader.cpp b/src/engine/cachingreader/cachingreader.cpp
index 2599d8bacd..56dae83ff5 100644
--- a/src/engine/cachingreader/cachingreader.cpp
+++ b/src/engine/cachingreader/cachingreader.cpp
@@ -57,12 +57,12 @@ CachingReader::CachingReader(QString group,
           // The capacity of the back channel must be equal to the number of
           // allocated chunks, because the worker use writeBlocking(). Otherwise
           // the worker could get stuck in a hot loop!!!
-          m_readerStatusFIFO(kNumberOfCachedChunksInMemory),
-          m_readerStatus(INVALID),
+          m_stateFIFO(kNumberOfCachedChunksInMemory),
+          m_state(State::Idle),
           m_mruCachingReaderChunk(nullptr),
           m_lruCachingReaderChunk(nullptr),
           m_sampleBuffer(CachingReaderChunk::kSamples * kNumberOfCachedChunksInMemory),
-          m_worker(group, &m_chunkReadRequestFIFO, &m_readerStatusFIFO) {
+          m_worker(group, &m_chunkReadRequestFIFO, &m_stateFIFO) {
 
     m_allocatedCachingReaderChunks.reserve(kNumberOfCachedChunksInMemory);
     // Divide up the allocated raw memory buffer into total_chunks
@@ -203,15 +203,42 @@ CachingReaderChunkForOwner* CachingReader::lookupChunkAndFreshen(SINT chunkIndex
 }
 
 void CachingReader::newTrack(TrackPointer pTrack) {
+    // Feed the track to the worker as soon as possible
+    // to get ready while the reader switches its internal
+    // state. There are no race conditions, because the
+    // reader polls the worker.
     m_worker.newTrack(pTrack);
     m_worker.workReady();
+    // Don't accept any new read requests until the current
+    // track has been unloaded and the new track has been
+    // loaded.
+    m_state = State::TrackLoading;
+    // Free all chunks with sample data from the current track.
+    freeAllChunks();
 }
 
 void CachingReader::process() {
     ReaderStatusUpdate update;
-    while (m_readerStatusFIFO.read(&update, 1) == 1) {
+    while (m_stateFIFO.read(&update, 1) == 1) {
+        DEBUG_ASSERT(m_state != State::Idle);
         auto pChunk = update.takeFromWorker();
         if (pChunk) {
+            // Result of a read request (with a chunk)
+            DEBUG_ASSERT(
+                    update.status == CHUNK_READ_SUCCESS ||
+                    update.status == CHUNK_READ_EOF ||
+                    update.status == CHUNK_READ_INVALID ||
+                    update.status == CHUNK_READ_DISCARDED);
+            if (m_state == State::TrackLoading) {
+                // All chunks have been freed before loading the next track!
+                DEBUG_ASSERT(!m_mruCachingReaderChunk);
+                DEBUG_ASSERT(!m_lruCachingReaderChunk);
+                // Discard all results from pending read requests for the
+                // previous track before the next track has been loaded.
+                freeChunk(pChunk);
+                continue;
+            }
+            DEBUG_ASSERT(m_state == State::TrackLoaded);
             if (update.status == CHUNK_READ_SUCCESS) {
                 // Insert or freshen the chunk in the MRU/LRU list after
                 // obtaining ownership from the worker.
@@ -220,24 +247,24 @@ void CachingReader::process() {
                 // Discard chunks that don't carry any data
                 freeChunk(pChunk);
             }
-        }
-        if (update.status == TRACK_NOT_LOADED) {
-            m_readerStatus = update.status;
-        } else if (update.status == TRACK_LOADED) {
-            m_readerStatus = update.status;
-            // Reset the max. readable frame index
-            m_readableFrameIndexRange = update.readableFrameIndexRange();
-            // Free all chunks with sample data from a previous track
-            freeAllChunks();
-        }
-        if (m_readerStatus == TRACK_LOADED) {
-            // Adjust the readable frame index range after loading or reading
-            m_readableFrameIndexRange = intersect(
-                    m_readableFrameIndexRange,
-                    update.readableFrameIndexRange());
+            // Adjust the readable frame index range (if available)
+            if (update.status != CHUNK_READ_DISCARDED) {
+                m_readableFrameIndexRange = intersect(
+                        m_readableFrameIndexRange,
+                        update.readableFrameIndexRange());
+            }
         } else {
+            // State update (without a chunk)
+            DEBUG_ASSERT(!m_mruCachingReaderChunk);
+            DEBUG_ASSERT(!m_lruCachingReaderChunk);
+            if (update.status == TRACK_LOADED) {
+                m_state = State::TrackLoaded;
+            } else {
+                DEBUG_ASSERT(update.status == TRACK_UNLOADED);
+                m_state = State::Idle;
+            }
             // Reset the readable frame index range
-            m_readableFrameIndexRange = mixxx::IndexRange();
+            m_readableFrameIndexRange = update.readableFrameIndexRange();
         }
     }
 }
@@ -261,7 +288,7 @@ CachingReader::ReadResult CachingReader::read(SINT startSample, SINT numSamples,
     }
 
     // If no track is loaded, don't do anything.
-    if (m_readerStatus != TRACK_LOADED) {
+    if (m_state != State::TrackLoaded) {
         return ReadResult::UNAVAILABLE;
     }
 
@@ -458,7 +485,7 @@ CachingReader::ReadResult CachingReader::read(SINT startSample, SINT numSamples,
 
 void CachingReader::hintAndMaybeWake(const HintVector& hintList) {
     // If no file is loaded, skip.
-    if (m_readerStatus != TRACK_LOADED) {
+    if (m_state != State::TrackLoaded) {
         return;
     }
 
diff --git a/src/engine/cachingreader/cachingreader.h b/src/engine/cachingreader/cachingreader.h
index 82a86b8d9a..2e981a5d1d 100644
--- a/src/engine/cachingreader/cachingreader.h
+++ b/src/engine/cachingreader/cachingreader.h
@@ -124,7 +124,7 @@ class CachingReader : public QObject {
     // Thread-safe FIFOs for communication between the engine callback and
     // reader thread.
     FIFO<CachingReaderChunkReadRequest> m_chunkReadRequestFIFO;
-    FIFO<ReaderStatusUpdate> m_readerStatusFIFO;
+    FIFO<ReaderStatusUpdate> m_stateFIFO;
 
     // Looks for the provided chunk number in the index of in-memory chunks and
     // returns it if it is present. If not, returns nullptr. If it is present then
@@ -151,7 +151,12 @@ class CachingReader : public QObject {
     // Gets a chunk from the free list, frees the LRU CachingReaderChunk if none available.
     CachingReaderChunkForOwner* allocateChunkExpireLRU(SINT chunkIndex);
 
-    ReaderStatus m_readerStatus;
+    enum class State {
+        Idle,
+        TrackLoading,
+        TrackLoaded,
+    };
+    State m_state;
 
     // Keeps track of all CachingReaderChunks we've allocated.
     QVector<CachingReaderChunkForOwner*> m_chunks;
diff --git a/src/engine/cachingreader/cachingreaderworker.cpp b/src/engine/cachingreader/cachingreaderworker.cpp
index 777601d2a6..09188b762c 100644
--- a/src/engine/cachingreader/cachingreaderworker.cpp
+++ b/src/engine/cachingreader/cachingreaderworker.cpp
@@ -122,19 +122,26 @@ void CachingReaderWorker::run() {
 }
 
 void CachingReaderWorker::loadTrack(const TrackPointer& pTrack) {
-    ReaderStatusUpdate update;
-    update.init(TRACK_NOT_LOADED);
+    // Discard all pending read requests
+    CachingReaderChunkReadRequest request;
+    while (m_pChunkReadRequestFIFO->read(&request, 1) == 1) {
+        const auto update = ReaderStatusUpdate::readDiscarded(request.chunk);
+        m_pReaderStatusFIFO->writeBlocking(&update, 1);
+    }
+
+    // Unload the track
+    m_readableFrameIndexRange = mixxx::IndexRange();
+    m_pAudioSource.reset(); // Close open file handles
 
     if (!pTrack) {
-        // Unload track
-        m_pAudioSource.reset(); // Close open file handles
-        m_readableFrameIndexRange = mixxx::IndexRange();
+        // If no new track is available then we are done
+        const auto update = ReaderStatusUpdate::trackNotLoaded();
         m_pReaderStatusFIFO->writeBlocking(&update, 1);
         return;
     }
 
     // Emit that a new track is loading, stops the current track
-    emit(trackLoading());
+    emit trackLoading();
 
     QString filename = pTrack->getLocation();
     if (filename.isEmpty() || !pTrack->checkFileExists()) {
@@ -142,10 +149,11 @@ void CachingReaderWorker::loadTrack(const TrackPointer& pTrack) {
                  << m_group
                  << "File not found"
                  << filename;
+        const auto update = ReaderStatusUpdate::trackNotLoaded();
         m_pReaderStatusFIFO->writeBlocking(&update, 1);
-        emit(trackLoadFailed(
+        emit trackLoadFailed(
             pTrack, QString("The file '%1' could not be found.")
-                    .arg(QDir::toNativeSeparators(filename))));
+                    .arg(QDir::toNativeSeparators(filename)));
         return;
     }
 
@@ -153,42 +161,50 @@ void CachingReaderWorker::loadTrack(const TrackPointer& pTrack) {
     config.setChannelCount(CachingReaderChunk::kChannels);
     m_pAudioSource = SoundSourceProxy(pTrack).openAudioSource(config);
     if (!m_pAudioSource) {
-        m_readableFrameIndexRange = mixxx::IndexRange();
         kLogger.warning()
                 << m_group
                 << "Failed to open file"
                 << filename;
+        const auto update = ReaderStatusUpdate::trackNotLoaded();
         m_pReaderStatusFIFO->writeBlocking(&update, 1);
-        emit(trackLoadFailed(
-            pTrack, QString("The file '%1' could not be loaded.").arg(filename)));
+        emit trackLoadFailed(
+            pTrack, QString("The file '%1' could not be loaded").arg(filename));
         return;
     }
 
-    const SINT tempReadBufferSize = m_pAudioSource->frames2samples(CachingReaderChunk::kFrames);
-    if (m_tempReadBuffer.size() != tempReadBufferSize) {
-        mixxx::SampleBuffer(tempReadBufferSize).swap(m_tempReadBuffer);
-    }
-
     // Initially assume that the complete content offered by audio source
     // is available for reading. Later if read errors occur this value will
     // be decreased to avoid repeated reading of corrupt audio data.
     m_readableFrameIndexRange = m_pAudioSource->frameIndexRange();
-
-    update.init(TRACK_LOADED, nullptr, m_pAudioSource->frameIndexRange());
-    m_pReaderStatusFIFO->writeBlocking(&update, 1);
-
-    // Clear the chunks to read list.
-    CachingReaderChunkReadRequest request;
-    while (m_pChunkReadRequestFIFO->read(&request, 1) == 1) {
-        update.init(CHUNK_READ_INVALID, request.chunk);
+    if (m_readableFrameIndexRange.empty()) {
+        m_pAudioSource.reset(); // Close open file handles
+        kLogger.warning()
+                << m_group
+                << "Failed to open empty file"
+                << filename;
+        const auto update = ReaderStatusUpdate::trackNotLoaded();
         m_pReaderStatusFIFO->writeBlocking(&update, 1);
+        emit trackLoadFailed(
+            pTrack, QString("The file '%1' is empty and could not be loaded").arg(filename));
+        return;
     }
 
+    // Adjust the internal buffer
+    const SINT tempReadBufferSize =
+            m_pAudioSource->frames2samples(CachingReaderChunk::kFrames);
+    if (m_tempReadBuffer.size() != tempReadBufferSize) {
+        mixxx::SampleBuffer(tempReadBufferSize).swap(m_tempReadBuffer);
+    }
+
+    const auto update =
+        ReaderStatusUpdate::trackLoaded(m_readableFrameIndexRange);
+    m_pReaderStatusFIFO->writeBlocking(&update, 1);
+
     // Emit that the track is loaded.
     const SINT sampleCount =
             CachingReaderChunk::frames2samples(
-                    m_pAudioSource->frameLength());
-    emit(trackLoaded(pTrack, m_pAudioSource->sampleRate(), sampleCount));
+                    m_readableFrameIndexRange.length());
+    emit trackLoaded(pTrack, m_pAudioSource->sampleRate(), sampleCount);
 }
 
 void CachingReaderWorker::quitWait() {
diff --git a/src/engine/cachingreader/cachingreaderworker.h b/src/engine/cachingreader/cachingreaderworker.h
index a7a9b2c584..6661677a34 100644
--- a/src/engine/cachingreader/cachingreaderworker.h
+++ b/src/engine/cachingreader/cachingreaderworker.h
@@ -26,12 +26,12 @@ typedef struct CachingReaderChunkReadRequest {
 } CachingReaderChunkReadRequest;
 
 enum ReaderStatus {
-    INVALID,
-    TRACK_NOT_LOADED,
     TRACK_LOADED,
+    TRACK_UNLOADED,
     CHUNK_READ_SUCCESS,
     CHUNK_READ_EOF,
-    CHUNK_READ_INVALID
+    CHUNK_READ_INVALID,
+    CHUNK_READ_DISCARDED, // response without frame index range!
 };
 
 // POD with trivial ctor/dtor/copy for passing through FIFO
@@ -45,15 +45,36 @@ typedef struct ReaderStatusUpdate {
     ReaderStatus status;
 
     void init(
-            ReaderStatus statusArg = INVALID,
-            CachingReaderChunk* chunkArg = nullptr,
-            const mixxx::IndexRange& readableFrameIndexRangeArg = mixxx::IndexRange()) {
+            ReaderStatus statusArg,
+            CachingReaderChunk* chunkArg,
+            const mixxx::IndexRange& readableFrameIndexRangeArg) {
         status = statusArg;
         chunk = chunkArg;
         readableFrameIndexRangeStart = readableFrameIndexRangeArg.start();
         readableFrameIndexRangeEnd = readableFrameIndexRangeArg.end();
     }
 
+    static ReaderStatusUpdate readDiscarded(
+            CachingReaderChunk* chunk) {
+        ReaderStatusUpdate update;
+        update.init(CHUNK_READ_DISCARDED, chunk, mixxx::IndexRange());
+        return update;
+    }
+
+    static ReaderStatusUpdate trackLoaded(
+            const mixxx::IndexRange& readableFrameIndexRange) {
+        DEBUG_ASSERT(!readableFrameIndexRange.empty());
+        ReaderStatusUpdate update;
+        update.init(TRACK_LOADED, nullptr, readableFrameIndexRange);
+        return update;
+    }
+
+    static ReaderStatusUpdate trackNotLoaded() {
+        ReaderStatusUpdate update;
+        update.init(TRACK_UNLOADED, nullptr, mixxx::IndexRange());
+        return update;
+    }
+
     CachingReaderChunkForOwner* takeFromWorker() {
         CachingReaderChunkForOwner* pChunk = nullptr;
         if (chunk) {
diff --git a/src/library/library.cpp b/src/library/library.cpp
index e8225b4d43..70f94efafb 100644
--- a/src/library/library.cpp
+++ b/src/library/library.cpp
@@ -563,14 +563,11 @@ void Library::setEditMedatataSelectedClick(bool enabled) {
     emit(setSelectedClick(enabled));
 }
 
-void Library::saveCachedTrack(Track* pTrack) noexcept {
+void Library::saveEvictedTrack(Track* pTrack) noexcept {
     // It can produce dangerous signal loops if the track is still
     // sending signals while being saved!
     // See: https://bugs.launchpad.net/mixxx/+bug/1365708
-    // NOTE(uklotzde, 2018-02-03): Simply disconnecting all receivers
-    // doesn't seem to work reliably. Emitting the clean() signal from
-    // a track that is about to deleted may cause access violations!!
-    pTrack->blockSignals(true);
+    DEBUG_ASSERT(pTrack->signalsBlocked());
 
     // The metadata must be exported while the cache is locked to
     // ensure that we have exclusive (write) access on the file
diff --git a/src/library/library.h b/src/library/library.h
index 6cf8c6a540..cd7c9c666d 100644
--- a/src/library/library.h
+++ b/src/library/library.h
@@ -147,7 +147,7 @@ class Library: public QObject,
 
   private:
     // Callback for GlobalTrackCache
-    void saveCachedTrack(Track* pTrack) noexcept override;
+    void saveEvictedTrack(Track* pTrack) noexcept override;
 
     const UserSettingsPointer m_pConfig;
 
diff --git a/src/test/globaltrackcache_test.cpp b/src/test/globaltrackcache_test.cpp
index 87f2025b12..b65055edf7 100644
--- a/src/test/globaltrackcache_test.cpp
+++ b/src/test/globaltrackcache_test.cpp
@@ -27,8 +27,11 @@ class TrackTitleThread: public QThread {
 
     void run() override {
         int loopCount = 0;
-        while (!m_stop.load()) {
+        while (!(m_stop.load() && GlobalTrackCacheLocker().isEmpty())) {
+            // Drop the previous reference to avoid resolving the
+            // same track twice
             m_recentTrackPtr.reset();
+            // Try to resolve the next track by guessing the id
             const TrackId trackId(loopCount % 2);
             auto track = GlobalTrackCacheLocker().lookupTrackById(trackId);
             if (track) {
@@ -43,10 +46,17 @@ class TrackTitleThread: public QThread {
                 }
                 ASSERT_TRUE(track->isDirty());
             }
+            // Replace the current reference with this one and keep it alive
+            // until the next loop cycle
             m_recentTrackPtr = std::move(track);
             ++loopCount;
         }
-        m_recentTrackPtr.reset();
+        // If the cache is empty all references must have been dropped.
+        // Why? m_recentTrackPtr is only valid if a pointer has been found
+        // in the cache during the previous cycle, i.e. the cache could not
+        // have been empty. In this case at least another loop cycle follow,
+        // and so on...
+        ASSERT_TRUE(!m_recentTrackPtr);
         qDebug() << "Finished" << loopCount << " thread loops";
     }
 
@@ -56,17 +66,23 @@ class TrackTitleThread: public QThread {
     std::atomic<bool> m_stop;
 };
 
+void deleteTrack(Track* pTrack) {
+    // Delete track objects directly in unit tests with
+    // no main event loop
+    delete pTrack;
+};
+
 } // anonymous namespace
 
 class GlobalTrackCacheTest: public MixxxTest, public virtual GlobalTrackCacheSaver {
   public:
-    void saveCachedTrack(Track* pTrack) noexcept override {
+    void saveEvictedTrack(Track* pTrack) noexcept override {
         ASSERT_FALSE(pTrack == nullptr);
     }
 
   protected:
     GlobalTrackCacheTest() {
-        GlobalTrackCache::createInstance(this);
+        GlobalTrackCache::createInstance(this, deleteTrack);
     }
     ~GlobalTrackCacheTest() {
         GlobalTrackCache::destroyInstance();
@@ -156,19 +172,27 @@ TEST_F(GlobalTrackCacheTest, concurrentDelete) {
 
         // lp1744550: Accessing the track from multiple threads is
         // required to cause a SIGSEGV
-        track->setArtist(track->getTitle());
+        track->setArtist(QString("Artist %1").arg(QString::number(i)));
 
         m_recentTrackPtr = std::move(track);
+
+        // Lookup the track again
+        track = GlobalTrackCacheLocker().lookupTrackById(trackId);
+        EXPECT_TRUE(static_cast<bool>(track));
+
+        // Ensure that track objects are evicted and deleted
+        QCoreApplication::processEvents();
     }
     m_recentTrackPtr.reset();
 
     workerThread.stop();
-    workerThread.wait();
 
     // Ensure that all track objects have been deleted
-    QCoreApplication::processEvents();
+    while (!GlobalTrackCacheLocker().isEmpty()) {
+        QCoreApplication::processEvents();
+    }
 
-    EXPECT_TRUE(GlobalTrackCacheLocker().isEmpty());
+    workerThread.wait();
 }
 
 TEST_F(GlobalTrackCacheTest, evictWhileMoving) {
@@ -184,4 +208,8 @@ TEST_F(GlobalTrackCacheTest, evictWhileMoving) {
 
     EXPECT_TRUE(static_cast<bool>(track1));
     EXPECT_FALSE(static_cast<bool>(track2));
+
+    track1.reset();
+
+    EXPECT_TRUE(GlobalTrackCacheLocker().isEmpty());
 }
diff --git a/src/test/librarytest.cpp b/src/test/librarytest.cpp
new file mode 100644
index 0000000000..8451597a52
--- /dev/null
+++ b/src/test/librarytest.cpp
@@ -0,0 +1,36 @@
+#include "test/librarytest.h"
+
+namespace {
+
+const bool kInMemoryDbConnection = true;
+
+void deleteTrack(Track* pTrack) {
+    // Delete track objects directly in unit tests with
+    // no main event loop
+    delete pTrack;
+};
+
+}
+
+LibraryTest::LibraryTest()
+    : m_mixxxDb(config(), kInMemoryDbConnection),
+      m_dbConnectionPooler(m_mixxxDb.connectionPool()),
+      m_dbConnection(mixxx::DbConnectionPooled(m_mixxxDb.connectionPool())),
+      m_pTrackCollection(std::make_unique<TrackCollection>(config())) {
+    MixxxDb::initDatabaseSchema(m_dbConnection);
+    m_pTrackCollection->connectDatabase(m_dbConnection);
+    GlobalTrackCache::createInstance(this, deleteTrack);
+}
+
+LibraryTest::~LibraryTest() {
+    m_pTrackCollection->disconnectDatabase();
+    m_pTrackCollection.reset();
+    // With the track collection all remaining track references
+    // should have been dropped before destroying the cache.
+    GlobalTrackCache::destroyInstance();
+}
+
+void LibraryTest::saveEvictedTrack(Track* pTrack) noexcept {
+    m_pTrackCollection->exportTrackMetadata(pTrack);
+    m_pTrackCollection->saveTrack(pTrack);
+}
diff --git a/src/test/librarytest.h b/src/test/librarytest.h
index 7cf4459a21..e97fcbe020 100644
--- a/src/test/librarytest.h
+++ b/src/test/librarytest.h
@@ -1,5 +1,6 @@
-#ifndef LIBRARYTEST_H
-#define LIBRARYTEST_H
+#pragma once
+
+#include <memory>
 
 #include "test/mixxxtest.h"
 
@@ -9,33 +10,15 @@
 #include "util/db/dbconnectionpooled.h"
 #include "track/globaltrackcache.h"
 
-namespace {
-    const bool kInMemoryDbConnection = true;
-} // anonymous namespace
-
 class LibraryTest : public MixxxTest,
     public virtual /*implements*/ GlobalTrackCacheSaver {
 
   public:
-    void saveCachedTrack(Track* pTrack) noexcept override {
-        m_trackCollection.exportTrackMetadata(pTrack);
-        m_trackCollection.saveTrack(pTrack);
-    }
+    void saveEvictedTrack(Track* pTrack) noexcept override;
 
   protected:
-    LibraryTest()
-        : m_mixxxDb(config(), kInMemoryDbConnection),
-          m_dbConnectionPooler(m_mixxxDb.connectionPool()),
-          m_dbConnection(mixxx::DbConnectionPooled(m_mixxxDb.connectionPool())),
-          m_trackCollection(config()) {
-        MixxxDb::initDatabaseSchema(m_dbConnection);
-        m_trackCollection.connectDatabase(m_dbConnection);
-        GlobalTrackCache::createInstance(this);
-    }
-    ~LibraryTest() override {
-        GlobalTrackCache::destroyInstance();
-        m_trackCollection.disconnectDatabase();
-    }
+    LibraryTest();
+    ~LibraryTest() override;
 
     mixxx::DbConnectionPoolPtr dbConnectionPool() const {
         return m_mixxxDb.connectionPool();
@@ -46,15 +29,12 @@ class LibraryTest : public MixxxTest,
     }
 
     TrackCollection* collection() {
-        return &m_trackCollection;
+        return m_pTrackCollection.get();
     }
 
   private:
     const MixxxDb m_mixxxDb;
     const mixxx::DbConnectionPooler m_dbConnectionPooler;
     QSqlDatabase m_dbConnection;
-    TrackCollection m_trackCollection;
+    std::unique_ptr<TrackCollection> m_pTrackCollection;
 };
-
-
-#endif /* LIBRARYTEST_H */
diff --git a/src/track/globaltrackcache.cpp b/src/track/globaltrackcache.cpp
index d608ed269e..1bb0cab342 100644
--- a/src/track/globaltrackcache.cpp
+++ b/src/track/globaltrackcache.cpp
@@ -36,18 +36,22 @@ TrackRef createTrackRef(const Track& track) {
 
 class EvictAndSaveFunctor {
   public:
-    explicit EvictAndSaveFunctor(GlobalTrackCacheEntryPointer cacheEntryPtr)
+    explicit EvictAndSaveFunctor(
+            GlobalTrackCacheEntryPointer cacheEntryPtr)
         : m_cacheEntryPtr(std::move(cacheEntryPtr)) {
     }
 
     void operator()(Track* plainPtr) {
         Q_UNUSED(plainPtr); // only used in DEBUG_ASSERT
+        DEBUG_ASSERT(m_cacheEntryPtr);
         DEBUG_ASSERT(plainPtr == m_cacheEntryPtr->getPlainPtr());
         // Here we move m_cacheEntryPtr and the owned track out of the
         // functor and the owning reference counting object.
         // This is required to break a cycle reference from the weak pointer
         // inside the cache entry to the same reference counting object.
         GlobalTrackCache::evictAndSaveCachedTrack(std::move(m_cacheEntryPtr));
+        // Verify that this functor is only invoked once
+        DEBUG_ASSERT(!m_cacheEntryPtr);
     }
 
     const GlobalTrackCacheEntryPointer& getCacheEntryPointer() const {
@@ -58,26 +62,6 @@ class EvictAndSaveFunctor {
     GlobalTrackCacheEntryPointer m_cacheEntryPtr;
 };
 
-
-void deleteTrack(Track* plainPtr) {
-    DEBUG_ASSERT(plainPtr);
-
-    // We safely delete the object via the Qt event queue instead
-    // of using operator delete! 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 (traceLogEnabled()) {
-        plainPtr->dumpObjectInfo();
-    }
-    if (debugLogEnabled()) {
-        kLogger.debug()
-                << "Deleting"
-                << plainPtr;
-    }
-    plainPtr->deleteLater();
-}
-
 } // anonymous namespace
 
 GlobalTrackCacheLocker::GlobalTrackCacheLocker()
@@ -210,14 +194,21 @@ void GlobalTrackCacheResolver::initTrackIdAndUnlockCache(TrackId trackId) {
 }
 
 //static
-void GlobalTrackCache::createInstance(GlobalTrackCacheSaver* pDeleter) {
+void GlobalTrackCache::createInstance(
+        GlobalTrackCacheSaver* pSaver,
+        deleteTrackFn_t deleteTrackFn) {
     DEBUG_ASSERT(!s_pInstance);
-    s_pInstance = new GlobalTrackCache(pDeleter);
+    s_pInstance = new GlobalTrackCache(pSaver, deleteTrackFn);
 }
 
 //static
 void GlobalTrackCache::destroyInstance() {
     DEBUG_ASSERT(s_pInstance);
+    // Processing all pending events is required to evict all
+    // remaining references from the cache.
+    QCoreApplication::processEvents();
+    // Now the cache should be empty
+    DEBUG_ASSERT(GlobalTrackCacheLocker().isEmpty());
     GlobalTrackCache* pInstance = s_pInstance;
     // Reset the static/global pointer before entering the destructor
     s_pInstance = nullptr;
@@ -226,6 +217,32 @@ void GlobalTrackCache::destroyInstance() {
     pInstance->deleteLater();
 }
 
+void GlobalTrackCacheEntry::TrackDeleter::operator()(Track* pTrack) const {
+    DEBUG_ASSERT(pTrack);
+
+    // We safely delete the object via the Qt event queue instead
+    // of using operator delete! 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 (traceLogEnabled()) {
+        pTrack->dumpObjectInfo();
+    }
+    if (debugLogEnabled()) {
+        kLogger.debug()
+                << "Deleting"
+                << pTrack;
+    }
+
+    if (m_deleteTrackFn) {
+        // Custom delete function
+        (*m_deleteTrackFn)(pTrack);
+    } else {
+        // Default delete function
+        pTrack->deleteLater();
+    }
+}
+
 //static
 void GlobalTrackCache::evictAndSaveCachedTrack(GlobalTrackCacheEntryPointer cacheEntryPtr) {
     // Any access to plainPtr before a validity check inside the
@@ -254,9 +271,12 @@ void GlobalTrackCache::evictAndSaveCachedTrack(GlobalTrackCacheEntryPointer cach
     }
 }
 
-GlobalTrackCache::GlobalTrackCache(GlobalTrackCacheSaver* pSaver)
+GlobalTrackCache::GlobalTrackCache(
+        GlobalTrackCacheSaver* pSaver,
+        deleteTrackFn_t deleteTrackFn)
     : m_mutex(QMutex::Recursive),
       m_pSaver(pSaver),
+      m_deleteTrackFn(deleteTrackFn),
       m_tracksById(kUnorderedCollectionMinCapacity, DbId::hash_fun) {
     DEBUG_ASSERT(m_pSaver);
     qRegisterMetaType<GlobalTrackCacheEntryPointer>("GlobalTrackCacheEntryPointer");
@@ -321,6 +341,18 @@ void GlobalTrackCache::relocateTracks(
     m_tracksByCanonicalLocation = std::move(relocatedTracksByCanonicalLocation);
 }
 
+void GlobalTrackCache::saveEvictedTrack(Track* pEvictedTrack) const {
+    DEBUG_ASSERT(pEvictedTrack);
+    // Disconnect all receivers and block signals before saving the
+    // track.
+    // NOTE(uklotzde, 2018-02-03): Simply disconnecting all receivers
+    // doesn't seem to work reliably. Emitting the clean() signal from
+    // a track that is about to deleted may cause access violations!!
+    pEvictedTrack->disconnect();
+    pEvictedTrack->blockSignals(true);
+    m_pSaver->saveEvictedTrack(pEvictedTrack);
+}
+
 void GlobalTrackCache::deactivate() {
     // Ideally the cache should be empty when destroyed.
     // But since this is difficult to achieve all remaining
@@ -331,7 +363,7 @@ void GlobalTrackCache::deactivate() {
     auto i = m_tracksById.begin();
     while (i != m_tracksById.end()) {
         Track* plainPtr= i->second->getPlainPtr();
-        m_pSaver->saveCachedTrack(plainPtr);
+        saveEvictedTrack(plainPtr);
         m_tracksByCanonicalLocation.erase(plainPtr->getCanonicalLocation());
         i = m_tracksById.erase(i);
     }
@@ -339,7 +371,7 @@ void GlobalTrackCache::deactivate() {
     auto j = m_tracksByCanonicalLocation.begin();
     while (j != m_tracksByCanonicalLocation.end()) {
         Track* plainPtr= j->second->getPlainPtr();
-        m_pSaver->saveCachedTrack(plainPtr);
+        saveEvictedTrack(plainPtr);
         j = m_tracksByCanonicalLocation.erase(j);
     }
 
@@ -413,13 +445,14 @@ TrackPointer GlobalTrackCache::lookupByRef(
 TrackPointer GlobalTrackCache::revive(
         GlobalTrackCacheEntryPointer entryPtr) {
 
-    TrackPointer savingPtr = entryPtr->getSavingWeakPtr().lock();
+    TrackPointer savingPtr = entryPtr->lock();
     if (savingPtr) {
         if (traceLogEnabled()) {
             kLogger.trace()
                     << "Found alive track"
                     << entryPtr->getPlainPtr();
         }
+        DEBUG_ASSERT(!savingPtr->signalsBlocked());
         return savingPtr;
     }
 
@@ -433,11 +466,12 @@ TrackPointer GlobalTrackCache::revive(
                 << "Reviving zombie track"
                 << entryPtr->getPlainPtr();
     }
-    DEBUG_ASSERT(entryPtr->getSavingWeakPtr().expired());
+    DEBUG_ASSERT(entryPtr->expired());
 
     savingPtr = TrackPointer(entryPtr->getPlainPtr(),
             EvictAndSaveFunctor(entryPtr));
-    entryPtr->setSavingWeakPtr(savingPtr);
+    entryPtr->init(savingPtr);
+    DEBUG_ASSERT(!savingPtr->signalsBlocked());
     return savingPtr;
 }
 
@@ -510,25 +544,19 @@ void GlobalTrackCache::resolve(
                 << "Cache miss - allocating track"
                 << trackRef;
     }
-    auto deletingPtr = std::unique_ptr<Track, void (&)(Track*)>(
+    auto deletingPtr = std::unique_ptr<Track, GlobalTrackCacheEntry::TrackDeleter>(
             new Track(
                     std::move(fileInfo),
                     std::move(pSecurityToken),
                     std::move(trackId)),
-            deleteTrack);
-    // Track objects live together with the cache on the main thread
-    // and will be deleted later within the event loop. But this
-    // 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 target thread.
-    deletingPtr->moveToThread(QApplication::instance()->thread());
+            GlobalTrackCacheEntry::TrackDeleter(m_deleteTrackFn));
 
     auto cacheEntryPtr = std::make_shared<GlobalTrackCacheEntry>(
             std::move(deletingPtr));
     auto savingPtr = TrackPointer(
             cacheEntryPtr->getPlainPtr(),
             EvictAndSaveFunctor(cacheEntryPtr));
-    cacheEntryPtr->setSavingWeakPtr(savingPtr);
+    cacheEntryPtr->init(savingPtr);
 
     if (debugLogEnabled()) {
         kLogger.debug()
@@ -553,6 +581,14 @@ void GlobalTrackCache::resolve(
                 trackRef.getCanonicalLocation(),
                 cacheEntryPtr));
     }
+
+    // Track objects live together with the cache on the main thread
+    // and will be deleted later within the event loop. But this
+    // 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());
+
     pCacheResolver->initLookupResult(
             GlobalTrackCacheLookupResult::MISS,
             std::move(savingPtr),
@@ -608,7 +644,7 @@ void GlobalTrackCache::evictAndSave(
 
     GlobalTrackCacheLocker cacheLocker;
 
-    if (!cacheEntryPtr->getSavingWeakPtr().expired()) {
+    if (!cacheEntryPtr->expired()) {
         // We have handed out (revived) this track again after our reference count
         // drops to zero and before acquiring the lock at the beginning of this function
         if (debugLogEnabled()) {
@@ -619,31 +655,36 @@ void GlobalTrackCache::evictAndSave(
         return;
     }
 
-    if (!evict(cacheEntryPtr->getPlainPtr())) {
+    if (!tryEvict(cacheEntryPtr->getPlainPtr())) {
         // A second deleter has already evicted the track from cache after our
         // reference count drops to zero and before acquiring the lock at the
         // beginning of this function
         if (debugLogEnabled()) {
             kLogger.debug()
-                    << "Skip to save an already evicted track"
+                    << "Skip to save an already evicted track again"
                     << cacheEntryPtr->getPlainPtr();
         }
         return;
     }
 
-    DEBUG_ASSERT(isEvicted(cacheEntryPtr->getPlainPtr()));
-    m_pSaver->saveCachedTrack(cacheEntryPtr->getPlainPtr());
+    DEBUG_ASSERT(!isCached(cacheEntryPtr->getPlainPtr()));
+    saveEvictedTrack(cacheEntryPtr->getPlainPtr());
 
-    // here the cacheEntryPtr goes out of scope, the cache is deleted
-    // including the owned track
+    // here the cacheEntryPtr goes out of scope, the cache entry is
+    // deleted including the owned track
 }
 
-bool GlobalTrackCache::evict(Track* plainPtr) {
+bool GlobalTrackCache::tryEvict(Track* plainPtr) {
     DEBUG_ASSERT(plainPtr);
     // Make the cached track object invisible to avoid reusing
     // it before starting to save it. Thi