From a26066d70f898b204034c5574404c3a0faf73bc4 Mon Sep 17 00:00:00 2001 From: Jonas Kvinge Date: Thu, 21 Nov 2024 15:43:33 +0100 Subject: [PATCH] Disconnect tagreader reply metaobject connection in lambda Otherwise the tagreader reply is deleted to early causing crashes. --- src/core/mainwindow.cpp | 12 ++++++++++-- src/covermanager/albumcoverchoicecontroller.cpp | 8 +++++++- src/covermanager/albumcovermanager.cpp | 10 ++++++++-- src/dialogs/edittagdialog.cpp | 8 +++++++- src/playlist/playlist.cpp | 6 +++++- src/tagreader/tagreaderclient.cpp | 6 ++---- src/tagreader/tagreaderloadcoverdatareply.cpp | 3 --- src/tagreader/tagreaderloadcoverimagereply.cpp | 3 --- src/tagreader/tagreaderreadfilereply.cpp | 3 --- src/tagreader/tagreaderreply.cpp | 2 -- 10 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/core/mainwindow.cpp b/src/core/mainwindow.cpp index 7ac7b18c0..c2ecdb93e 100644 --- a/src/core/mainwindow.cpp +++ b/src/core/mainwindow.cpp @@ -2197,7 +2197,11 @@ void MainWindow::RenumberTracks() { song.set_track(track); TagReaderReplyPtr reply = app_->tagreader_client()->WriteFileAsync(song.url().toLocalFile(), song); QPersistentModelIndex persistent_index = QPersistentModelIndex(source_index); - QObject::connect(&*reply, &TagReaderReply::Finished, this, [this, reply, persistent_index]() { SongSaveComplete(reply, persistent_index); }, Qt::QueuedConnection); + SharedPtr connection = make_shared(); + *connection = QObject::connect(&*reply, &TagReaderReply::Finished, this, [this, reply, persistent_index, connection]() { + SongSaveComplete(reply, persistent_index); + QObject::disconnect(*connection); + }, Qt::QueuedConnection); } ++track; } @@ -2228,7 +2232,11 @@ void MainWindow::SelectionSetValue() { if (song.url().isLocalFile() && Playlist::set_column_value(song, column, column_value)) { TagReaderReplyPtr reply = app_->tagreader_client()->WriteFileAsync(song.url().toLocalFile(), song); QPersistentModelIndex persistent_index = QPersistentModelIndex(source_index); - QObject::connect(&*reply, &TagReaderReply::Finished, this, [this, reply, persistent_index]() { SongSaveComplete(reply, persistent_index); }, Qt::QueuedConnection); + SharedPtr connection = make_shared(); + *connection = QObject::connect(&*reply, &TagReaderReply::Finished, this, [this, reply, persistent_index, connection]() { + SongSaveComplete(reply, persistent_index); + QObject::disconnect(*connection); + }, Qt::QueuedConnection); } else if (song.source() == Song::Source::Stream) { app_->playlist_manager()->current()->setData(source_index, column_value, 0); diff --git a/src/covermanager/albumcoverchoicecontroller.cpp b/src/covermanager/albumcoverchoicecontroller.cpp index c4328b9dd..38271c8a8 100644 --- a/src/covermanager/albumcoverchoicecontroller.cpp +++ b/src/covermanager/albumcoverchoicecontroller.cpp @@ -22,6 +22,7 @@ #include "config.h" #include +#include #include #include @@ -79,6 +80,7 @@ #include "coverfromurldialog.h" #include "currentalbumcoverloader.h" +using std::make_shared; using namespace Qt::Literals::StringLiterals; QSet *AlbumCoverChoiceController::sImageExtensions = nullptr; @@ -743,7 +745,11 @@ void AlbumCoverChoiceController::SaveCoverEmbeddedToSong(const Song &song, const cover_save_tasks_.append(song); const bool art_embedded = !image_data.isNull(); TagReaderReplyPtr reply = tagreader_client_->SaveCoverAsync(song.url().toLocalFile(), SaveTagCoverData(cover_filename, image_data, mime_type)); - QObject::connect(&*reply, &TagReaderReply::Finished, this, [this, reply, song, art_embedded]() { SaveEmbeddedCoverFinished(reply, song, art_embedded); }); + SharedPtr connection = make_shared(); + *connection = QObject::connect(&*reply, &TagReaderReply::Finished, this, [this, reply, song, art_embedded, connection]() { + SaveEmbeddedCoverFinished(reply, song, art_embedded); + QObject::disconnect(*connection); + }); } diff --git a/src/covermanager/albumcovermanager.cpp b/src/covermanager/albumcovermanager.cpp index 7abbccdf0..8da41dcac 100644 --- a/src/covermanager/albumcovermanager.cpp +++ b/src/covermanager/albumcovermanager.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -96,6 +97,7 @@ using namespace std::literals::chrono_literals; using namespace Qt::Literals::StringLiterals; +using std::make_shared; namespace { constexpr char kSettingsGroup[] = "CoverManager"; @@ -855,8 +857,10 @@ void AlbumCoverManager::SaveImageToAlbums(Song *song, const AlbumCoverImageResul for (const QUrl &url : std::as_const(album_item->urls)) { const bool art_embedded = !result.image_data.isEmpty(); TagReaderReplyPtr reply = tagreader_client_->SaveCoverAsync(url.toLocalFile(), SaveTagCoverData(result.image_data, result.mime_type)); - QObject::connect(&*reply, &TagReaderReply::Finished, this, [this, reply, album_item, url, art_embedded]() { + SharedPtr connection = make_shared(); + *connection = QObject::connect(&*reply, &TagReaderReply::Finished, this, [this, reply, album_item, url, art_embedded, connection]() { SaveEmbeddedCoverFinished(reply, album_item, url, art_embedded); + QObject::disconnect(*connection); }); cover_save_tasks_.insert(album_item, url); } @@ -1005,8 +1009,10 @@ void AlbumCoverManager::SaveAndSetCover(AlbumItem *album_item, const AlbumCoverI for (const QUrl &url : urls) { const bool art_embedded = !result.image_data.isEmpty(); TagReaderReplyPtr reply = tagreader_client_->SaveCoverAsync(url.toLocalFile(), SaveTagCoverData(result.cover_url.isValid() ? result.cover_url.toLocalFile() : QString(), result.image_data, result.mime_type)); - QObject::connect(&*reply, &TagReaderReply::Finished, this, [this, reply, album_item, url, art_embedded]() { + SharedPtr connection = std::make_shared(); + *connection = QObject::connect(&*reply, &TagReaderReply::Finished, this, [this, reply, album_item, url, art_embedded, connection]() { SaveEmbeddedCoverFinished(reply, album_item, url, art_embedded); + QObject::disconnect(*connection); }); cover_save_tasks_.insert(album_item, url); } diff --git a/src/dialogs/edittagdialog.cpp b/src/dialogs/edittagdialog.cpp index ef7856319..e7100f30e 100644 --- a/src/dialogs/edittagdialog.cpp +++ b/src/dialogs/edittagdialog.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -99,6 +100,7 @@ #include "edittagdialog.h" #include "ui_edittagdialog.h" +using std::make_shared; using namespace Qt::Literals::StringLiterals; namespace { @@ -1311,7 +1313,11 @@ void EditTagDialog::SaveData() { save_tags_options |= TagReaderClient::SaveOption::Cover; } TagReaderReplyPtr reply = tagreader_client_->WriteFileAsync(ref.current_.url().toLocalFile(), ref.current_, save_tags_options, save_tag_cover_data); - QObject::connect(&*reply, &TagReaderReply::Finished, this, [this, reply, ref]() { SongSaveTagsComplete(reply, ref.current_.url().toLocalFile(), ref.current_, ref.cover_action_); }, Qt::QueuedConnection); + SharedPtr connection = make_shared(); + *connection = QObject::connect(&*reply, &TagReaderReply::Finished, this, [this, reply, ref, connection]() { + SongSaveTagsComplete(reply, ref.current_.url().toLocalFile(), ref.current_, ref.cover_action_); + QObject::disconnect(*connection); + }, Qt::QueuedConnection); } // If the cover was changed, but no tags written, make sure to update the collection. else if (ref.cover_action_ != UpdateCoverAction::None && !ref.current_.effective_albumartist().isEmpty() && !ref.current_.album().isEmpty()) { diff --git a/src/playlist/playlist.cpp b/src/playlist/playlist.cpp index 0853b0bcd..f55fbe159 100644 --- a/src/playlist/playlist.cpp +++ b/src/playlist/playlist.cpp @@ -434,7 +434,11 @@ bool Playlist::setData(const QModelIndex &idx, const QVariant &value, const int if (song.url().isLocalFile()) { TagReaderReplyPtr reply = tagreader_client_->WriteFileAsync(song.url().toLocalFile(), song); QPersistentModelIndex persistent_index = QPersistentModelIndex(idx); - QObject::connect(&*reply, &TagReaderReply::Finished, this, [this, reply, persistent_index, item]() { SongSaveComplete(reply, persistent_index, item->OriginalMetadata()); }, Qt::QueuedConnection); + SharedPtr connection = make_shared(); + *connection = QObject::connect(&*reply, &TagReaderReply::Finished, this, [this, reply, persistent_index, item, connection]() { + SongSaveComplete(reply, persistent_index, item->OriginalMetadata()); + QObject::disconnect(*connection); + }, Qt::QueuedConnection); } else if (song.is_radio()) { item->SetMetadata(song); diff --git a/src/tagreader/tagreaderclient.cpp b/src/tagreader/tagreaderclient.cpp index 6ea57502a..45a25a615 100644 --- a/src/tagreader/tagreaderclient.cpp +++ b/src/tagreader/tagreaderclient.cpp @@ -384,8 +384,7 @@ void TagReaderClient::SaveSongsPlaycountAsync(const SongList &songs) { Q_ASSERT(QThread::currentThread() != thread()); for (const Song &song : songs) { - TagReaderReplyPtr reply = SaveSongPlaycountAsync(song.url().toLocalFile(), song.playcount()); - QObject::connect(&*reply, &TagReaderReply::Finished, &*reply, &TagReaderReply::deleteLater); + SaveSongPlaycountAsync(song.url().toLocalFile(), song.playcount()); } } @@ -418,8 +417,7 @@ void TagReaderClient::SaveSongsRatingAsync(const SongList &songs) { Q_ASSERT(QThread::currentThread() != thread()); for (const Song &song : songs) { - TagReaderReplyPtr reply = SaveSongRatingAsync(song.url().toLocalFile(), song.rating()); - QObject::connect(&*reply, &TagReaderReply::Finished, &*reply, &TagReaderReply::deleteLater); + SaveSongRatingAsync(song.url().toLocalFile(), song.rating()); } } diff --git a/src/tagreader/tagreaderloadcoverdatareply.cpp b/src/tagreader/tagreaderloadcoverdatareply.cpp index beb438e82..e8031eccb 100644 --- a/src/tagreader/tagreaderloadcoverdatareply.cpp +++ b/src/tagreader/tagreaderloadcoverdatareply.cpp @@ -40,7 +40,4 @@ void TagReaderLoadCoverDataReply::EmitFinished() { Q_EMIT TagReaderReply::Finished(filename_, result_); Q_EMIT TagReaderLoadCoverDataReply::Finished(filename_, data_, result_); - QObject::disconnect(this, &TagReaderReply::Finished, nullptr, nullptr); - QObject::disconnect(this, &TagReaderLoadCoverDataReply::Finished, nullptr, nullptr); - } diff --git a/src/tagreader/tagreaderloadcoverimagereply.cpp b/src/tagreader/tagreaderloadcoverimagereply.cpp index 35e3cc201..7d6b7bd41 100644 --- a/src/tagreader/tagreaderloadcoverimagereply.cpp +++ b/src/tagreader/tagreaderloadcoverimagereply.cpp @@ -40,7 +40,4 @@ void TagReaderLoadCoverImageReply::EmitFinished() { Q_EMIT TagReaderReply::Finished(filename_, result_); Q_EMIT TagReaderLoadCoverImageReply::Finished(filename_, image_, result_); - QObject::disconnect(this, &TagReaderReply::Finished, nullptr, nullptr); - QObject::disconnect(this, &TagReaderLoadCoverImageReply::Finished, nullptr, nullptr); - } diff --git a/src/tagreader/tagreaderreadfilereply.cpp b/src/tagreader/tagreaderreadfilereply.cpp index 04f5ec529..7503215e9 100644 --- a/src/tagreader/tagreaderreadfilereply.cpp +++ b/src/tagreader/tagreaderreadfilereply.cpp @@ -40,7 +40,4 @@ void TagReaderReadFileReply::EmitFinished() { Q_EMIT TagReaderReply::Finished(filename_, result_); Q_EMIT TagReaderReadFileReply::Finished(filename_, song_, result_); - QObject::disconnect(this, &TagReaderReply::Finished, nullptr, nullptr); - QObject::disconnect(this, &TagReaderReadFileReply::Finished, nullptr, nullptr); - } diff --git a/src/tagreader/tagreaderreply.cpp b/src/tagreader/tagreaderreply.cpp index eadd894f5..de0507a27 100644 --- a/src/tagreader/tagreaderreply.cpp +++ b/src/tagreader/tagreaderreply.cpp @@ -52,6 +52,4 @@ void TagReaderReply::EmitFinished() { Q_EMIT TagReaderReply::Finished(filename_, result_); - QObject::disconnect(this, &TagReaderReply::Finished, nullptr, nullptr); - }