Skip to content

Commit

Permalink
[sociald] Automatically time out stuck network requests. Contributes …
Browse files Browse the repository at this point in the history 
…to JB#16704

This commit adds code to time out network requests after a certain
period of time.  This ensures that msyncd won't keep the device from
entering suspend if a network request hangs.

It also fixes a problem in the Twitter Home Timeline (Posts) sync
adapter where a network error would cause the finished() handler to
be skipped, which would cause the semaphore to remain undecremented,
which caused the plugin to hang.

Contributes to JB#16704
  • Loading branch information
Chris Adams committed Apr 1, 2014
1 parent c405e28 commit d8dacdb
Show file tree
Hide file tree
Showing 13 changed files with 100 additions and 12 deletions.
2 changes: 2 additions & 0 deletions src/facebook/facebookcalendartypesyncadaptor.cpp
View file
Expand Up @@ -134,6 +134,7 @@ void FacebookCalendarTypeSyncAdaptor::requestEvents(int accountId, const QString

// we're requesting data. Increment the semaphore so that we know we're still busy.
incrementSemaphore(accountId);
setupReplyTimeout(accountId, reply);
} else {
TRACE(SOCIALD_ERROR,
QString(QLatin1String("error: unable to request events "\
Expand All @@ -150,6 +151,7 @@ void FacebookCalendarTypeSyncAdaptor::finishedHandler()

disconnect(reply);
reply->deleteLater();
removeReplyTimeout(accountId, reply);

bool ok = false;
QJsonObject parsed = parseJsonObjectReplyData(replyData, &ok);
Expand Down
2 changes: 2 additions & 0 deletions src/facebook/facebookcontactsyncadaptor.cpp
View file
Expand Up @@ -205,6 +205,7 @@ void FacebookContactSyncAdaptor::requestData(int accountId, const QString &acces

// we're requesting data. Increment the semaphore so that we know we're still busy.
incrementSemaphore(accountId);
setupReplyTimeout(accountId, reply);
} else {
TRACE(SOCIALD_ERROR,
QString(QLatin1String("error: unable to request %1 from Facebook account with id %2"))
Expand All @@ -223,6 +224,7 @@ void FacebookContactSyncAdaptor::friendsFinishedHandler()
QByteArray replyData = reply->readAll();
disconnect(reply);
reply->deleteLater();
removeReplyTimeout(accountId, reply);

bool ok = false;
QJsonObject parsed = parseJsonObjectReplyData(replyData, &ok);
Expand Down
3 changes: 0 additions & 3 deletions src/facebook/facebookdatatypesyncadaptor.cpp
View file
Expand Up @@ -192,9 +192,6 @@ void FacebookDataTypeSyncAdaptor::errorHandler(QNetworkReply::NetworkError err)
}
}
}

disconnect(reply);
reply->deleteLater();
}

void FacebookDataTypeSyncAdaptor::sslErrorsHandler(const QList<QSslError> &errs)
Expand Down
7 changes: 7 additions & 0 deletions src/facebook/facebookimagesyncadaptor.cpp
View file
Expand Up @@ -131,6 +131,7 @@ void FacebookImageSyncAdaptor::requestData(int accountId,

// we're requesting data. Increment the semaphore so that we know we're still busy.
incrementSemaphore(accountId);
setupReplyTimeout(accountId, reply);
} else {
TRACE(SOCIALD_ERROR,
QString(QLatin1String("error: unable to request data from Facebook account with id %1"))
Expand All @@ -151,6 +152,7 @@ void FacebookImageSyncAdaptor::albumsFinishedHandler()
QByteArray replyData = reply->readAll();
disconnect(reply);
reply->deleteLater();
removeReplyTimeout(accountId, reply);

bool ok = false;
QJsonObject parsed = parseJsonObjectReplyData(replyData, &ok);
Expand Down Expand Up @@ -256,6 +258,7 @@ void FacebookImageSyncAdaptor::imagesFinishedHandler()
QByteArray replyData = reply->readAll();
disconnect(reply);
reply->deleteLater();
removeReplyTimeout(accountId, reply);

bool ok = false;
QJsonObject parsed = parseJsonObjectReplyData(replyData, &ok);
Expand Down Expand Up @@ -398,6 +401,9 @@ void FacebookImageSyncAdaptor::possiblyAddNewUser(const QString &fbUserId, int a
connect(reply, SIGNAL(sslErrors(QList<QSslError>)),
this, SLOT(sslErrorsHandler(QList<QSslError>)));
connect(reply, SIGNAL(finished()), this, SLOT(userFinishedHandler()));

incrementSemaphore(accountId);
setupReplyTimeout(accountId, reply);
}
}

Expand All @@ -424,6 +430,7 @@ void FacebookImageSyncAdaptor::userFinishedHandler()
QString updatedStr = parsed.value(QLatin1String("updated_time")).toString();

m_db.addUser(fbUserId, QDateTime::fromString(updatedStr, Qt::ISODate), fbName);
decrementSemaphore(accountId);
}

void FacebookImageSyncAdaptor::initRemovalDetectionLists()
Expand Down
2 changes: 2 additions & 0 deletions src/facebook/facebooknotificationsyncadaptor.cpp
View file
Expand Up @@ -77,6 +77,7 @@ void FacebookNotificationSyncAdaptor::requestNotifications(int accountId, const

// we're requesting data. Increment the semaphore so that we know we're still busy.
incrementSemaphore(accountId);
setupReplyTimeout(accountId, reply);
} else {
TRACE(SOCIALD_ERROR,
QString(QLatin1String("error: unable to request notifications from Facebook account with id %1"))
Expand All @@ -92,6 +93,7 @@ void FacebookNotificationSyncAdaptor::finishedHandler()
QByteArray replyData = reply->readAll();
disconnect(reply);
reply->deleteLater();
removeReplyTimeout(accountId, reply);

bool ok = false;

Expand Down
4 changes: 4 additions & 0 deletions src/facebook/facebookpostsyncadaptor.cpp
View file
Expand Up @@ -139,6 +139,7 @@ void FacebookPostSyncAdaptor::requestMe(int accountId, const QString &accessToke

// we're requesting data. Increment the semaphore so that we know we're still busy.
incrementSemaphore(accountId);
setupReplyTimeout(accountId, reply);
} else {
TRACE(SOCIALD_ERROR,
QString(QLatin1String("error: unable to request feed posts from Facebook account with id %1"))
Expand Down Expand Up @@ -173,6 +174,7 @@ void FacebookPostSyncAdaptor::requestPosts(int accountId, const QString &accessT

// we're requesting data. Increment the semaphore so that we know we're still busy.
incrementSemaphore(accountId);
setupReplyTimeout(accountId, reply);
} else {
TRACE(SOCIALD_ERROR,
QString(QLatin1String("error: unable to request home posts from Facebook account with id %1"))
Expand All @@ -189,6 +191,7 @@ void FacebookPostSyncAdaptor::finishedMeHandler()
QByteArray replyData = reply->readAll();
disconnect(reply);
reply->deleteLater();
removeReplyTimeout(accountId, reply);

bool ok = false;
QJsonObject parsed = parseJsonObjectReplyData(replyData, &ok);
Expand Down Expand Up @@ -216,6 +219,7 @@ void FacebookPostSyncAdaptor::finishedPostsHandler()
QByteArray replyData = reply->readAll();
disconnect(reply);
reply->deleteLater();
removeReplyTimeout(accountId, reply);

bool ok = false;
QJsonObject parsed = parseJsonObjectReplyData(replyData, &ok);
Expand Down
10 changes: 9 additions & 1 deletion src/google/googlecalendarsyncadaptor.cpp
View file
Expand Up @@ -421,6 +421,8 @@ void GoogleCalendarSyncAdaptor::requestCalendars(int accountId, const QString &a
connect(reply, SIGNAL(sslErrors(QList<QSslError>)),
this, SLOT(sslErrorsHandler(QList<QSslError>)));
connect(reply, SIGNAL(finished()), this, SLOT(calendarsFinishedHandler()));

setupReplyTimeout(accountId, reply);
} else {
TRACE(SOCIALD_ERROR,
QString(QLatin1String("error: unable to request calendars"
Expand All @@ -442,6 +444,7 @@ void GoogleCalendarSyncAdaptor::calendarsFinishedHandler()

disconnect(reply);
reply->deleteLater();
removeReplyTimeout(accountId, reply);

// parse the calendars' metadata from the response.
bool fetchingNextPage = false;
Expand Down Expand Up @@ -625,6 +628,8 @@ void GoogleCalendarSyncAdaptor::requestEvents(int accountId, const QString &acce
TRACE(SOCIALD_DEBUG,
QString(QLatin1String("Requesting calendar events for Google account: %1: %2"))
.arg(accountId).arg(url.toString()));

setupReplyTimeout(accountId, reply);
} else {
TRACE(SOCIALD_ERROR,
QString(QLatin1String("error: unable to request events for calendar %1"
Expand All @@ -647,6 +652,7 @@ void GoogleCalendarSyncAdaptor::eventsFinishedHandler()

disconnect(reply);
reply->deleteLater();
removeReplyTimeout(accountId, reply);

bool fetchingNextPage = false;
bool ok = false;
Expand Down Expand Up @@ -916,6 +922,8 @@ void GoogleCalendarSyncAdaptor::upsyncChanges(int accountId, const QString &acce
this, SLOT(sslErrorsHandler(QList<QSslError>)));
connect(reply, SIGNAL(finished()), this, SLOT(upsyncFinishedHandler()));

setupReplyTimeout(accountId, reply);

TRACE(SOCIALD_DEBUG,
QString(QLatin1String("Upsyncing change: %1 to calendarId: %2 of account %3:\n%4\n%5\n"))
.arg(upsyncTypeStr).arg(calendarId).arg(accountId).arg(request.url().toString()).arg(QString::fromUtf8(eventData)));
Expand All @@ -933,7 +941,6 @@ void GoogleCalendarSyncAdaptor::upsyncFinishedHandler()
{
QNetworkReply *reply = qobject_cast<QNetworkReply*>(sender());
int accountId = reply->property("accountId").toInt();
QString accessToken = reply->property("accessToken").toString();
QString kcalEventId = reply->property("kcalEventId").toString();
QString calendarId = reply->property("calendarId").toString();
int upsyncType = reply->property("upsyncType").toInt();
Expand All @@ -948,6 +955,7 @@ void GoogleCalendarSyncAdaptor::upsyncFinishedHandler()

disconnect(reply);
reply->deleteLater();
removeReplyTimeout(accountId, reply);

// parse the calendars' metadata from the response.
if (isError) {
Expand Down
3 changes: 3 additions & 0 deletions src/google/googlecontactsyncadaptor.cpp
View file
Expand Up @@ -199,6 +199,7 @@ void GoogleContactSyncAdaptor::requestData(int accountId, const QString &accessT
connect(reply, SIGNAL(finished()), this, SLOT(contactsFinishedHandler()));
connect(reply, SIGNAL(error(QNetworkReply::NetworkError)), this, SLOT(errorHandler(QNetworkReply::NetworkError)));
connect(reply, SIGNAL(sslErrors(QList<QSslError>)), this, SLOT(sslErrorsHandler(QList<QSslError>)));
setupReplyTimeout(accountId, reply);
} else {
TRACE(SOCIALD_ERROR,
QString(QLatin1String("error: unable to request contacts from Google account with id %1"))
Expand All @@ -219,6 +220,8 @@ void GoogleContactSyncAdaptor::contactsFinishedHandler()
QDateTime lastSyncTimestamp = reply->property("lastSyncTimestamp").toDateTime();
bool isError = reply->property("isError").toBool();
reply->deleteLater();
removeReplyTimeout(accountId, reply);

if (isError) {
TRACE(SOCIALD_ERROR,
QString(QLatin1String("error occurred when performing contacts request for Google account %1"))
Expand Down
45 changes: 45 additions & 0 deletions src/socialnetworksyncadaptor.cpp
View file
Expand Up @@ -12,12 +12,14 @@
#include "trace.h"

#include <QtCore/QJsonDocument>
#include <QtCore/QTimer>
#include <QtSql/QSqlDatabase>
#include <QtSql/QSqlQuery>
#include <QtSql/QSqlError>
#include <QtSql/QSqlRecord>

#include <QtNetwork/QNetworkAccessManager>
#include <QtNetwork/QNetworkReply>

// sailfish-components-accounts-qt5
#include <accountmanager.h>
Expand Down Expand Up @@ -301,6 +303,49 @@ void SocialNetworkSyncAdaptor::decrementSemaphore(int accountId)
}
}

void SocialNetworkSyncAdaptor::timeoutReply()
{
QTimer *timer = qobject_cast<QTimer*>(sender());
QNetworkReply *reply = timer->property("networkReply").value<QNetworkReply*>();
int accountId = timer->property("accountId").toInt();

m_networkReplyTimeouts[accountId].remove(reply);
reply->setProperty("isError", QVariant::fromValue<bool>(true)); // just in case it finishes.
reply->disconnect();
reply->deleteLater();

TRACE(SOCIALD_ERROR,
QString(QLatin1String("network request timed out while performing sync with account %1"))
.arg(accountId));

decrementSemaphore(accountId);
}

void SocialNetworkSyncAdaptor::setupReplyTimeout(int accountId, QNetworkReply *reply)
{
// this function should be called whenever a new network request is performed.
QTimer *timer = new QTimer(this);
timer->setSingleShot(true);
timer->setInterval(60000);
timer->setProperty("accountId", accountId);
timer->setProperty("networkReply", QVariant::fromValue<QNetworkReply*>(reply));
connect(timer, SIGNAL(timeout()), this, SLOT(timeoutReply()));
timer->start();
m_networkReplyTimeouts[accountId].insert(reply, timer);
}

void SocialNetworkSyncAdaptor::removeReplyTimeout(int accountId, QNetworkReply *reply)
{
// this function should be called by the finished() handler for the reply.
QTimer *timer = m_networkReplyTimeouts[accountId].value(reply);
if (!reply) {
return;
}

delete timer;
m_networkReplyTimeouts[accountId].remove(reply);
}

QJsonObject SocialNetworkSyncAdaptor::parseJsonObjectReplyData(const QByteArray &replyData, bool *ok)
{
QJsonDocument jsonDocument = QJsonDocument::fromJson(replyData);
Expand Down
9 changes: 9 additions & 0 deletions src/socialnetworksyncadaptor.h
View file
Expand Up @@ -19,6 +19,8 @@
class QSqlDatabase;
class SyncService;
class QNetworkAccessManager;
class QTimer;
class QNetworkReply;
class AccountManager;
class SocialNetworkSyncDatabase;

Expand Down Expand Up @@ -69,13 +71,19 @@ class SocialNetworkSyncAdaptor : public QObject
void incrementSemaphore(int accountId);
void decrementSemaphore(int accountId);

// network reply timeouts
void setupReplyTimeout(int accountId, QNetworkReply *reply);
void removeReplyTimeout(int accountId, QNetworkReply *reply);

// Parsing methods
static QJsonObject parseJsonObjectReplyData(const QByteArray &replyData, bool *ok);
static QJsonArray parseJsonArrayReplyData(const QByteArray &replyData, bool *ok);

AccountManager *const accountManager;
QNetworkAccessManager * const networkAccessManager; // Do not allow the pointer to be changed

protected Q_SLOTS:
virtual void timeoutReply();

private:
SocialNetworkSyncDatabase *m_syncDb;
Expand All @@ -84,6 +92,7 @@ class SocialNetworkSyncAdaptor : public QObject
QString m_serviceName;
SyncService *m_syncService;
QMap<int, int> m_accountSyncSemaphores;
QMap<int, QMap<QNetworkReply*, QTimer *> > m_networkReplyTimeouts;
};

#endif // SOCIALNETWORKSYNCADAPTOR_H
19 changes: 11 additions & 8 deletions src/twitter/twitterdatatypesyncadaptor.cpp
View file
Expand Up @@ -196,8 +196,14 @@ void TwitterDataTypeSyncAdaptor::errorHandler(QNetworkReply::NetworkError err)
{
QNetworkReply *reply = qobject_cast<QNetworkReply*>(sender());
QByteArray replyData = reply->readAll();
disconnect(reply);
reply->deleteLater();
int accountId = reply->property("accountId").toInt();

TRACE(SOCIALD_ERROR,
QString(QLatin1String("error: %1 request with account %2 experienced error: %3"))
.arg(SyncService::dataType(dataType)).arg(accountId).arg(err));
// set "isError" on the reply so that adapters know to ignore the result in the finished() handler
reply->setProperty("isError", QVariant::fromValue<bool>(true));
// Note: not all errors are "unrecoverable" errors, so we don't change the status here.

bool ok = false;
QJsonObject parsed = parseJsonObjectReplyData(replyData, &ok);
Expand All @@ -208,7 +214,6 @@ void TwitterDataTypeSyncAdaptor::errorHandler(QNetworkReply::NetworkError err)
foreach (QJsonValue data, dataList) {
QJsonObject dataMap = data.toObject();
if (dataMap.value("code").toDouble() == 32) {
int accountId = sender()->property("accountId").toInt();
Account *account = accountManager->account(accountId);
if (account->status() == Account::Initialized) {
setCredentialsNeedUpdate(account);
Expand All @@ -219,11 +224,6 @@ void TwitterDataTypeSyncAdaptor::errorHandler(QNetworkReply::NetworkError err)
}
}
}
TRACE(SOCIALD_ERROR,
QString(QLatin1String("error: %1 request with account %2 experienced error: %3"))
.arg(SyncService::dataType(dataType)).arg(sender()->property("accountId").toInt()).arg(err));
// the error is an incomprehensible enum value, but that doesn't matter to users.
setStatus(SocialNetworkSyncAdaptor::Error);
}

void TwitterDataTypeSyncAdaptor::sslErrorsHandler(const QList<QSslError> &errs)
Expand All @@ -238,6 +238,9 @@ void TwitterDataTypeSyncAdaptor::sslErrorsHandler(const QList<QSslError> &errs)
TRACE(SOCIALD_ERROR,
QString(QLatin1String("error: %1 request with account %2 experienced ssl errors: %3"))
.arg(SyncService::dataType(dataType)).arg(sender()->property("accountId").toInt()).arg(sslerrs));
// set "isError" on the reply so that adapters know to ignore the result in the finished() handler
sender()->setProperty("isError", QVariant::fromValue<bool>(true));
// Note: not all errors are "unrecoverable" errors, so we don't change the status here.
}

// This function taken from http://qt-project.org/wiki/HMAC-SHA1 which is in the public domain
Expand Down
4 changes: 4 additions & 0 deletions src/twitter/twitterhometimelinesyncadaptor.cpp
View file
Expand Up @@ -73,6 +73,7 @@ void TwitterHomeTimelineSyncAdaptor::requestMe(int accountId, const QString &oau

// we're requesting data. Increment the semaphore so that we know we're still busy.
incrementSemaphore(accountId);
setupReplyTimeout(accountId, reply);
} else {
TRACE(SOCIALD_ERROR,
QString(QLatin1String("error: unable to request user verification from Twitter account with id %1"))
Expand Down Expand Up @@ -115,6 +116,7 @@ void TwitterHomeTimelineSyncAdaptor::requestPosts(int accountId, const QString &

// we're requesting data. Increment the semaphore so that we know we're still busy.
incrementSemaphore(accountId);
setupReplyTimeout(accountId, reply);
} else {
TRACE(SOCIALD_ERROR,
QString(QLatin1String("error: unable to request user timeline posts from Twitter account with id %1"))
Expand All @@ -137,6 +139,7 @@ void TwitterHomeTimelineSyncAdaptor::finishedMeHandler()
QByteArray replyData = reply->readAll();
disconnect(reply);
reply->deleteLater();
removeReplyTimeout(accountId, reply);

bool ok = false;
QJsonObject parsed = parseJsonObjectReplyData(replyData, &ok);
Expand Down Expand Up @@ -177,6 +180,7 @@ void TwitterHomeTimelineSyncAdaptor::finishedPostsHandler()
QByteArray replyData = reply->readAll();
disconnect(reply);
reply->deleteLater();
removeReplyTimeout(accountId, reply);

bool ok = false;
QJsonArray tweets = parseJsonArrayReplyData(replyData, &ok);
Expand Down

0 comments on commit d8dacdb

Please sign in to comment.