Funkwhale
/
Clementine-audio-player-Mac-Linux-Windows
mirror of https://github.com/clementine-player/Clementine
1
0
Fork 0
Code Issues Releases Wiki Activity

Fix gpodder sync memory leaks in success cases. 

A closure created by NewClosure that handles Qt signals is destroyed if the
signal object is destroyed, the slot object is destroyed, or the signal is
invoked. In the case where the sender is passed as a shared pointer, the
reference prevents the sender from being destroyed before the closure.

So for closures built to handle responses returned from ApiRequest in
GPodderSync, the closure object and the response object will only be destroyed
after the signal is invoked. In some cases, separate closures are built for
error signals as well. For these, only one closure will be destroyed. The other
closures and the response object will be leaked.

A simple fix for the success cases is to remove the unnecessary error case
closures and directly connect the signals to slots. This is low hanging fruit
and still leaves leaks in the error cases. Those cases will require a more
complete solution to properly manage the life cycle of the response object.
This commit is contained in:
Jim Broadus 2019-12-28 16:22:49 -08:00
parent 922e10bc48
commit 36179a7197
2 changed files with 23 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
src/internet/podcasts/gpoddersync.cpp
View File

@ -157,14 +157,17 @@ void GPodderSync::GetUpdatesNow() {
api_->deviceUpdates(username_, DeviceId(), timestamp));
NewClosure(reply, SIGNAL(finished()), this,
SLOT(DeviceUpdatesFinished(mygpo::DeviceUpdatesPtr)), reply);
NewClosure(reply, SIGNAL(parseError()), this,
SLOT(DeviceUpdatesFailed(mygpo::DeviceUpdatesPtr)), reply);
NewClosure(reply, SIGNAL(requestError(QNetworkReply::NetworkError)), this,
SLOT(DeviceUpdatesFailed(mygpo::DeviceUpdatesPtr)), reply);
connect(reply.data(), SIGNAL(parseError()), SLOT(DeviceUpdatesParseError()));
connect(reply.data(), SIGNAL(requestError(QNetworkReply::NetworkError)),
SLOT(DeviceUpdatesRequestError(QNetworkReply::NetworkError)));
}
void GPodderSync::DeviceUpdatesFailed(mygpo::DeviceUpdatesPtr reply) {
qLog(Warning) << "Failed to get gpodder.net device updates";
void GPodderSync::DeviceUpdatesParseError() {
qLog(Warning) << "Failed to get gpodder device updates: parse error";
}
void GPodderSync::DeviceUpdatesRequestError(QNetworkReply::NetworkError error) {
qLog(Warning) << "Failed to get gpodder device updates:" << error;
}
void GPodderSync::DeviceUpdatesFinished(mygpo::DeviceUpdatesPtr reply) {
@ -344,15 +347,19 @@ void GPodderSync::FlushUpdateQueue() {
NewClosure(reply, SIGNAL(finished()), this,
SLOT(AddRemoveFinished(mygpo::AddRemoveResultPtr, QList<QUrl>)),
reply, all_urls.toList());
NewClosure(reply, SIGNAL(parseError()), this,
SLOT(AddRemoveFailed(mygpo::AddRemoveResultPtr)), reply);
NewClosure(reply, SIGNAL(requestError(QNetworkReply::NetworkError)), this,
SLOT(AddRemoveFailed(mygpo::AddRemoveResultPtr)), reply);
connect(reply.data(), SIGNAL(parseError()), SLOT(AddRemoveParseError()));
connect(reply.data(), SIGNAL(requestError(QNetworkReply::NetworkError)),
SLOT(AddRemoveRequestError(QNetworkReply::NetworkError)));
}
void GPodderSync::AddRemoveFailed(mygpo::AddRemoveResultPtr reply) {
void GPodderSync::AddRemoveParseError() {
flushing_queue_ = false;
qLog(Warning) << "Failed to update gpodder.net subscriptions";
qLog(Warning) << "Failed to update gpodder subscriptions: parse error";
}
void GPodderSync::AddRemoveRequestError(QNetworkReply::NetworkError err) {
flushing_queue_ = false;
qLog(Warning) << "Failed to update gpodder subscriptions:" << err;
}
void GPodderSync::AddRemoveFinished(mygpo::AddRemoveResultPtr reply,

6
src/internet/podcasts/gpoddersync.h
View File

@ -77,7 +77,8 @@ class GPodderSync : public QObject {
const QString& password);
void DeviceUpdatesFinished(mygpo::DeviceUpdatesPtr reply);
void DeviceUpdatesFailed(mygpo::DeviceUpdatesPtr reply);
void DeviceUpdatesParseError();
void DeviceUpdatesRequestError(QNetworkReply::NetworkError error);
void NewPodcastLoaded(PodcastUrlLoaderReply* reply, const QUrl& url,
const QList<mygpo::EpisodePtr>& actions);
@ -91,7 +92,8 @@ class GPodderSync : public QObject {
void AddRemoveFinished(mygpo::AddRemoveResultPtr reply,
const QList<QUrl>& affected_urls);
void AddRemoveFailed(mygpo::AddRemoveResultPtr reply);
void AddRemoveParseError();
void AddRemoveRequestError(QNetworkReply::NetworkError error);
private:
void LoadQueue();