From 948ddecb5ca9a67d9f7107da5a93899ea4f86211 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sat, 29 Feb 2020 12:46:48 +0100 Subject: [PATCH 1/2] Fix potential race conditions and crashes during shutdown --- src/library/trackcollectionmanager.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/library/trackcollectionmanager.cpp b/src/library/trackcollectionmanager.cpp index 7808e985b428..1500f9a9bb4b 100644 --- a/src/library/trackcollectionmanager.cpp +++ b/src/library/trackcollectionmanager.cpp @@ -1,5 +1,7 @@ #include "library/trackcollectionmanager.h" +#include + #include "library/externaltrackcollection.h" #include "library/scanner/libraryscanner.h" #include "library/trackcollection.h" @@ -17,6 +19,8 @@ const QString kConfigGroup = QStringLiteral("[TrackCollection]"); const ConfigKey kConfigKeyRepairDatabaseOnNextRestart(kConfigGroup, "RepairDatabaseOnNextRestart"); +constexpr int kThreadPoolTimeoutMillisOnShutdown = 30000; // 30 sec + inline parented_ptr createInternalTrackCollection( TrackCollectionManager* parent, @@ -136,9 +140,23 @@ TrackCollectionManager::~TrackCollectionManager() { kLogger.warning() << "BaseTrackCache is still in use"; } + // Ensure that all pending, concurrent tasks have been either + // cancelled or finished to prevent access to tracks in + // GlobalTrackCache before it gets deactivated. + kLogger.info() + << "Stopping all pending tasks in worker threads"; + QThreadPool::globalInstance()->clear(); + if (!QThreadPool::globalInstance()->waitForDone( + kThreadPoolTimeoutMillisOnShutdown)) { + kLogger.warning() + << "Failed to stop pending tasks in worker threads"; + } + // Evict all remaining tracks from the cache to trigger // updating of modified tracks. We assume that no other // components are accessing those files at this point. + kLogger.info() + << "Deactivating global track cache"; GlobalTrackCacheLocker().deactivateCache(); for (const auto& externalCollection : m_externalCollections) { @@ -153,6 +171,8 @@ TrackCollectionManager::~TrackCollectionManager() { m_pInternalCollection->disconnectDatabase(); + kLogger.info() + << "Destroying global track cache"; GlobalTrackCache::destroyInstance(); } From ede8b698b3dd8ee8313d52ddc5ed30eb827512a7 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sat, 29 Feb 2020 23:00:51 +0100 Subject: [PATCH 2/2] Explain rationale behind constant --- src/library/trackcollectionmanager.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/library/trackcollectionmanager.cpp b/src/library/trackcollectionmanager.cpp index 1500f9a9bb4b..df4aae952eae 100644 --- a/src/library/trackcollectionmanager.cpp +++ b/src/library/trackcollectionmanager.cpp @@ -19,6 +19,9 @@ const QString kConfigGroup = QStringLiteral("[TrackCollection]"); const ConfigKey kConfigKeyRepairDatabaseOnNextRestart(kConfigGroup, "RepairDatabaseOnNextRestart"); +// Prevent infinite delays during shutdown. Long enough to finish +// the pending tasks and short enough to prevent users from killing +// the process. constexpr int kThreadPoolTimeoutMillisOnShutdown = 30000; // 30 sec inline