diff --git a/src/App.cpp b/src/App.cpp --- a/src/App.cpp +++ b/src/App.cpp @@ -74,6 +74,7 @@ #include #include #include //slotConfigShortcuts() +#include #include #include @@ -215,6 +216,11 @@ AmarokConfig::self()->save(); #endif + // wait for threads to finish + ThreadWeaver::Queue::instance()->requestAbort(); + ThreadWeaver::Queue::instance()->finish(); + ThreadWeaver::Queue::instance()->shutDown(); + ScriptManager::destroy(); // this must be deleted before the connection to the Xserver is diff --git a/src/browsers/CollectionTreeItemModelBase.h b/src/browsers/CollectionTreeItemModelBase.h --- a/src/browsers/CollectionTreeItemModelBase.h +++ b/src/browsers/CollectionTreeItemModelBase.h @@ -37,7 +37,6 @@ class Collection; } class CollectionTreeItem; -class QMutex; class QTimeLine; class TrackLoaderJob; @@ -145,13 +144,8 @@ void handleNormalQueryResult( Collections::QueryMaker *qm, const Meta::DataList &dataList ); Collections::QueryMaker::QueryType mapCategoryToQueryType( int levelType ) const; + void tracksLoaded( const Meta::AlbumPtr &album, const QModelIndex &index, const Meta::TrackList &tracks ); - /** - * This function is thread-safe - */ - void tracksLoaded( Meta::AlbumPtr album, const QModelIndex &index, const Meta::TrackList &tracks ); - - QMutex *m_loadingAlbumsMutex; QHash m_years; mutable QSet m_loadingAlbums; diff --git a/src/browsers/CollectionTreeItemModelBase.cpp b/src/browsers/CollectionTreeItemModelBase.cpp --- a/src/browsers/CollectionTreeItemModelBase.cpp +++ b/src/browsers/CollectionTreeItemModelBase.cpp @@ -40,13 +40,16 @@ #include #include -#include #include +#include #include #include #include #include +#include + + using namespace Meta; @@ -57,22 +60,40 @@ : m_index( index ) , m_album( album ) , m_model( model ) + , m_abortRequested( false ) + { + if( !m_model || !m_album || !m_index.isValid() ) + requestAbort(); + } + + void requestAbort() override { + m_abortRequested = true; } protected: void run( ThreadWeaver::JobPointer self, ThreadWeaver::Thread *thread ) override { Q_UNUSED( self ) Q_UNUSED( thread ) - m_model->tracksLoaded( m_album, m_index, m_album->tracks() ); + if( m_abortRequested || !m_model ) + return; + + const auto tracks = m_album->tracks(); + + if( m_model && !m_abortRequested ) + { + auto slot = std::bind( &CollectionTreeItemModelBase::tracksLoaded, m_model, m_album, m_index, tracks ); + QTimer::singleShot( 0, m_model, slot ); + } } private: QModelIndex m_index; Meta::AlbumPtr m_album; - CollectionTreeItemModelBase *m_model; + QPointer m_model; + bool m_abortRequested; }; inline uint qHash( const Meta::DataPtr &data ) @@ -89,7 +110,6 @@ CollectionTreeItemModelBase::CollectionTreeItemModelBase( ) : QAbstractItemModel() - , m_loadingAlbumsMutex( new QMutex ) , m_rootItem( 0 ) , m_animFrame( 0 ) , m_loading1( QPixmap( QStandardPaths::locate( QStandardPaths::GenericDataLocation, "amarok/images/loading1.png" ) ) ) @@ -113,8 +133,6 @@ if( m_rootItem ) m_rootItem->deleteLater(); - - delete m_loadingAlbumsMutex; } Qt::ItemFlags CollectionTreeItemModelBase::flags(const QModelIndex & index) const @@ -125,7 +143,6 @@ flags = Qt::ItemIsEnabled | Qt::ItemIsSelectable | Qt::ItemIsDragEnabled | Qt::ItemIsEditable; } return flags; - } bool @@ -289,7 +306,6 @@ } else if( !album->name().isEmpty() ) { - QMutexLocker locker( m_loadingAlbumsMutex ); if( !m_loadingAlbums.contains( album ) ) { m_loadingAlbums.insert( album ); @@ -327,7 +343,6 @@ else if( !album->name().isEmpty() ) { - QMutexLocker locker( m_loadingAlbumsMutex ); if( !m_loadingAlbums.contains( album ) ) { m_loadingAlbums.insert( album ); @@ -680,14 +695,13 @@ } void -CollectionTreeItemModelBase::tracksLoaded( Meta::AlbumPtr album, const QModelIndex &index, const Meta::TrackList& tracks ) +CollectionTreeItemModelBase::tracksLoaded( const Meta::AlbumPtr &album, const QModelIndex &index, const Meta::TrackList& tracks ) { DEBUG_BLOCK if( !album ) return; - QMutexLocker locker( m_loadingAlbumsMutex ); m_loadingAlbums.remove( album ); if( !index.isValid() ) @@ -704,15 +718,11 @@ debug() << "Valid album year found:" << year; } - // Set the year in the thread associated with this - auto lambda = [=] () { - if( !m_years.contains( album.data() ) || m_years.value( album.data() ) != year ) - { - m_years[ album.data() ] = year; - emit dataChanged( index, index ); - } - }; - QTimer::singleShot( 0, this, lambda ); + if( !m_years.contains( album.data() ) || m_years.value( album.data() ) != year ) + { + m_years[ album.data() ] = year; + emit dataChanged( index, index ); + } } void diff --git a/src/core-impl/collections/db/sql/SqlCollection.h b/src/core-impl/collections/db/sql/SqlCollection.h --- a/src/core-impl/collections/db/sql/SqlCollection.h +++ b/src/core-impl/collections/db/sql/SqlCollection.h @@ -100,7 +100,7 @@ QSharedPointer m_sqlStorage; SqlScanResultProcessor* m_scanProcessor; - AbstractDirectoryWatcher* m_directoryWatcher; + QWeakPointer m_directoryWatcher; SqlCollectionLocationFactory* m_collectionLocationFactory; SqlQueryMakerFactory* m_queryMakerFactory; diff --git a/src/core-impl/collections/db/sql/SqlCollection.cpp b/src/core-impl/collections/db/sql/SqlCollection.cpp --- a/src/core-impl/collections/db/sql/SqlCollection.cpp +++ b/src/core-impl/collections/db/sql/SqlCollection.cpp @@ -221,12 +221,12 @@ SqlCollection::SqlCollection( QSharedPointer storage ) : DatabaseCollection() - , m_registry( 0 ) + , m_registry( nullptr ) , m_sqlStorage( storage ) - , m_scanProcessor( 0 ) - , m_directoryWatcher( 0 ) - , m_collectionLocationFactory( 0 ) - , m_queryMakerFactory( 0 ) + , m_scanProcessor( nullptr ) + , m_directoryWatcher( nullptr ) + , m_collectionLocationFactory( nullptr ) + , m_queryMakerFactory( nullptr ) { qRegisterMetaType( "TrackUrls" ); qRegisterMetaType( "ChangedTrackUrls" ); @@ -270,19 +270,22 @@ // scanning m_scanManager = new SqlScanManager( this, this ); m_scanProcessor = new SqlScanResultProcessor( m_scanManager, this, this ); - m_directoryWatcher = new SqlDirectoryWatcher( this ); - connect( m_directoryWatcher, &AbstractDirectoryWatcher::done, - m_directoryWatcher, &AbstractDirectoryWatcher::deleteLater ); // auto delete - connect( m_directoryWatcher, &AbstractDirectoryWatcher::requestScan, + auto directoryWatcher = QSharedPointer::create( this ); + m_directoryWatcher = directoryWatcher.toWeakRef(); + connect( directoryWatcher.data(), &AbstractDirectoryWatcher::done, + directoryWatcher.data(), &AbstractDirectoryWatcher::deleteLater ); // auto delete + connect( directoryWatcher.data(), &AbstractDirectoryWatcher::requestScan, m_scanManager, &GenericScanManager::requestScan ); - ThreadWeaver::Queue::instance()->enqueue( QSharedPointer(m_directoryWatcher) ); + ThreadWeaver::Queue::instance()->enqueue( directoryWatcher ); } SqlCollection::~SqlCollection() { DEBUG_BLOCK - m_directoryWatcher->abort(); + if( auto directoryWatcher = m_directoryWatcher.toStrongRef() ) + directoryWatcher->requestAbort(); + delete m_scanProcessor; // this prevents any further commits from the scanner delete m_collectionLocationFactory; delete m_queryMakerFactory; diff --git a/src/scanner/AbstractDirectoryWatcher.h b/src/scanner/AbstractDirectoryWatcher.h --- a/src/scanner/AbstractDirectoryWatcher.h +++ b/src/scanner/AbstractDirectoryWatcher.h @@ -58,7 +58,7 @@ AbstractDirectoryWatcher(); void run(ThreadWeaver::JobPointer self = QSharedPointer(), ThreadWeaver::Thread *thread = 0) override; - void abort(); + void requestAbort() override; /** Pauses the emitting of the scan signal */ void setBlockScanning( bool block ); diff --git a/src/scanner/AbstractDirectoryWatcher.cpp b/src/scanner/AbstractDirectoryWatcher.cpp --- a/src/scanner/AbstractDirectoryWatcher.cpp +++ b/src/scanner/AbstractDirectoryWatcher.cpp @@ -138,7 +138,7 @@ } void -AbstractDirectoryWatcher::abort() +AbstractDirectoryWatcher::requestAbort() { DEBUG_BLOCK