cleanup race on deleteLater after callback processed

This commit is contained in:
Stephen Birarda 2018-06-26 16:25:05 -07:00
parent 70cab27c1a
commit b5254f1ea5
13 changed files with 66 additions and 106 deletions

View file

@ -660,9 +660,8 @@ void DomainGatekeeper::requestUserPublicKey(const QString& username, bool isOpti
// even if we have a public key for them right now, request a new one in case it has just changed
JSONCallbackParameters callbackParams;
callbackParams.jsonCallbackReceiver = this;
callbackParams.callbackReceiver = this;
callbackParams.jsonCallbackMethod = "publicKeyJSONCallback";
callbackParams.errorCallbackReceiver = this;
callbackParams.errorCallbackMethod = "publicKeyJSONErrorCallback";
@ -893,9 +892,8 @@ void DomainGatekeeper::getGroupMemberships(const QString& username) {
JSONCallbackParameters callbackParams;
callbackParams.jsonCallbackReceiver = this;
callbackParams.callbackReceiver = this;
callbackParams.jsonCallbackMethod = "getIsGroupMemberJSONCallback";
callbackParams.errorCallbackReceiver = this;
callbackParams.errorCallbackMethod = "getIsGroupMemberErrorCallback";
const QString GET_IS_GROUP_MEMBER_PATH = "api/v1/groups/members/%2";
@ -960,9 +958,8 @@ void DomainGatekeeper::getIsGroupMemberErrorCallback(QNetworkReply* requestReply
void DomainGatekeeper::getDomainOwnerFriendsList() {
JSONCallbackParameters callbackParams;
callbackParams.jsonCallbackReceiver = this;
callbackParams.callbackReceiver = this;
callbackParams.jsonCallbackMethod = "getDomainOwnerFriendsListJSONCallback";
callbackParams.errorCallbackReceiver = this;
callbackParams.errorCallbackMethod = "getDomainOwnerFriendsListErrorCallback";
const QString GET_FRIENDS_LIST_PATH = "api/v1/user/friends";

View file

@ -514,7 +514,7 @@ void DomainServer::getTemporaryName(bool force) {
// request a temporary name from the metaverse
auto accountManager = DependencyManager::get<AccountManager>();
JSONCallbackParameters callbackParameters { this, "handleTempDomainSuccess", this, "handleTempDomainError" };
JSONCallbackParameters callbackParameters { this, "handleTempDomainSuccess", "handleTempDomainError" };
accountManager->sendRequest("/api/v1/domains/temporary", AccountManagerAuth::None,
QNetworkAccessManager::PostOperation, callbackParameters);
}
@ -1345,7 +1345,7 @@ void DomainServer::sendPendingTransactionsToServer() {
JSONCallbackParameters transactionCallbackParams;
transactionCallbackParams.jsonCallbackReceiver = this;
transactionCallbackParams.callbackReceiver = this;
transactionCallbackParams.jsonCallbackMethod = "transactionJSONCallback";
while (i != _pendingAssignmentCredits.end()) {
@ -1449,7 +1449,7 @@ void DomainServer::sendHeartbeatToMetaverse(const QString& networkAddress) {
DependencyManager::get<AccountManager>()->sendRequest(DOMAIN_UPDATE.arg(uuidStringWithoutCurlyBraces(getID())),
AccountManagerAuth::Optional,
QNetworkAccessManager::PutOperation,
JSONCallbackParameters(nullptr, QString(), this, "handleMetaverseHeartbeatError"),
JSONCallbackParameters(this, QString(), "handleMetaverseHeartbeatError"),
domainUpdateJSON.toUtf8());
}
@ -1531,9 +1531,8 @@ void DomainServer::sendICEServerAddressToMetaverseAPI() {
// make sure we hear about failure so we can retry
JSONCallbackParameters callbackParameters;
callbackParameters.errorCallbackReceiver = this;
callbackParameters.callbackReceiver = this;
callbackParameters.errorCallbackMethod = "handleFailedICEServerAddressUpdate";
callbackParameters.jsonCallbackReceiver = this;
callbackParameters.jsonCallbackMethod = "handleSuccessfulICEServerAddressUpdate";
static bool printedIceServerMessage = false;

View file

@ -1815,9 +1815,8 @@ void DomainServerSettingsManager::apiRefreshGroupInformation() {
void DomainServerSettingsManager::apiGetGroupID(const QString& groupName) {
JSONCallbackParameters callbackParams;
callbackParams.jsonCallbackReceiver = this;
callbackParams.callbackReceiver = this;
callbackParams.jsonCallbackMethod = "apiGetGroupIDJSONCallback";
callbackParams.errorCallbackReceiver = this;
callbackParams.errorCallbackMethod = "apiGetGroupIDErrorCallback";
const QString GET_GROUP_ID_PATH = "api/v1/groups/names/%1";
@ -1882,9 +1881,8 @@ void DomainServerSettingsManager::apiGetGroupIDErrorCallback(QNetworkReply* requ
void DomainServerSettingsManager::apiGetGroupRanks(const QUuid& groupID) {
JSONCallbackParameters callbackParams;
callbackParams.jsonCallbackReceiver = this;
callbackParams.callbackReceiver = this;
callbackParams.jsonCallbackMethod = "apiGetGroupRanksJSONCallback";
callbackParams.errorCallbackReceiver = this;
callbackParams.errorCallbackMethod = "apiGetGroupRanksErrorCallback";
const QString GET_GROUP_RANKS_PATH = "api/v1/groups/%1/ranks";

View file

@ -97,7 +97,7 @@ void DiscoverabilityManager::updateLocation() {
locationObject.insert(AVAILABILITY_KEY_IN_LOCATION, findableByString(static_cast<Discoverability::Mode>(_mode.get())));
JSONCallbackParameters callbackParameters;
callbackParameters.jsonCallbackReceiver = this;
callbackParameters.callbackReceiver = this;
callbackParameters.jsonCallbackMethod = "handleHeartbeatResponse";
// figure out if we'll send a fresh location or just a simple heartbeat
@ -121,7 +121,7 @@ void DiscoverabilityManager::updateLocation() {
// we still send a heartbeat to the metaverse server for stats collection
JSONCallbackParameters callbackParameters;
callbackParameters.jsonCallbackReceiver = this;
callbackParameters.callbackReceiver = this;
callbackParameters.jsonCallbackMethod = "handleHeartbeatResponse";
accountManager->sendRequest(API_USER_HEARTBEAT_PATH, AccountManagerAuth::Optional,

View file

@ -68,7 +68,7 @@ Handler(updateItem)
void Ledger::send(const QString& endpoint, const QString& success, const QString& fail, QNetworkAccessManager::Operation method, AccountManagerAuth::Type authType, QJsonObject request) {
auto accountManager = DependencyManager::get<AccountManager>();
const QString URL = "/api/v1/commerce/";
JSONCallbackParameters callbackParams(this, success, this, fail);
JSONCallbackParameters callbackParams(this, success, fail);
qCInfo(commerce) << "Sending" << endpoint << QJsonDocument(request).toJson(QJsonDocument::Compact);
accountManager->sendRequest(URL + endpoint,
authType,

View file

@ -113,9 +113,8 @@ void LoginDialog::linkSteam() {
}
JSONCallbackParameters callbackParams;
callbackParams.jsonCallbackReceiver = this;
callbackParams.callbackReceiver = this;
callbackParams.jsonCallbackMethod = "linkCompleted";
callbackParams.errorCallbackReceiver = this;
callbackParams.errorCallbackMethod = "linkFailed";
const QString LINK_STEAM_PATH = "api/v1/user/steam/link";
@ -141,9 +140,8 @@ void LoginDialog::createAccountFromStream(QString username) {
}
JSONCallbackParameters callbackParams;
callbackParams.jsonCallbackReceiver = this;
callbackParams.callbackReceiver = this;
callbackParams.jsonCallbackMethod = "createCompleted";
callbackParams.errorCallbackReceiver = this;
callbackParams.errorCallbackMethod = "createFailed";
const QString CREATE_ACCOUNT_FROM_STEAM_PATH = "api/v1/user/steam/create";
@ -204,9 +202,8 @@ void LoginDialog::createFailed(QNetworkReply* reply) {
void LoginDialog::signup(const QString& email, const QString& username, const QString& password) {
JSONCallbackParameters callbackParams;
callbackParams.jsonCallbackReceiver = this;
callbackParams.callbackReceiver = this;
callbackParams.jsonCallbackMethod = "signupCompleted";
callbackParams.errorCallbackReceiver = this;
callbackParams.errorCallbackMethod = "signupFailed";
QJsonObject payload;

View file

@ -493,7 +493,7 @@ void Snapshot::uploadSnapshot(const QString& filename, const QUrl& href) {
multiPart->append(imagePart);
auto accountManager = DependencyManager::get<AccountManager>();
JSONCallbackParameters callbackParams(uploader, "uploadSuccess", uploader, "uploadFailure");
JSONCallbackParameters callbackParams(uploader, "uploadSuccess", "uploadFailure");
accountManager->sendRequest(SNAPSHOT_UPLOAD_URL, AccountManagerAuth::Required, QNetworkAccessManager::PostOperation,
callbackParams, nullptr, multiPart);

View file

@ -60,7 +60,7 @@ void SnapshotUploader::uploadSuccess(QNetworkReply* reply) {
rootObject.insert("user_story", userStoryObject);
auto accountManager = DependencyManager::get<AccountManager>();
JSONCallbackParameters callbackParams(this, "createStorySuccess", this, "createStoryFailure");
JSONCallbackParameters callbackParams(this, "createStorySuccess", "createStoryFailure");
accountManager->sendRequest(STORY_UPLOAD_URL,
AccountManagerAuth::Required,

View file

@ -47,15 +47,12 @@ Q_DECLARE_METATYPE(JSONCallbackParameters)
const QString ACCOUNTS_GROUP = "accounts";
JSONCallbackParameters::JSONCallbackParameters(QObject* jsonCallbackReceiver, const QString& jsonCallbackMethod,
QObject* errorCallbackReceiver, const QString& errorCallbackMethod,
QObject* updateReceiver, const QString& updateSlot) :
jsonCallbackReceiver(jsonCallbackReceiver),
JSONCallbackParameters::JSONCallbackParameters(QObject* callbackReceiver,
const QString& jsonCallbackMethod,
const QString& errorCallbackMethod) :
callbackReceiver(callbackReceiver),
jsonCallbackMethod(jsonCallbackMethod),
errorCallbackReceiver(errorCallbackReceiver),
errorCallbackMethod(errorCallbackMethod),
updateReceiver(updateReceiver),
updateSlot(updateSlot)
errorCallbackMethod(errorCallbackMethod)
{
}
@ -323,54 +320,50 @@ void AccountManager::sendRequest(const QString& path,
}
}
if (!callbackParams.isEmpty()) {
if (callbackParams.updateReceiver && !callbackParams.updateSlot.isEmpty()) {
callbackParams.updateReceiver->connect(networkReply, SIGNAL(uploadProgress(qint64, qint64)),
callbackParams.updateSlot.toStdString().c_str());
connect(networkReply, &QNetworkReply::finished, this, [this, networkReply] {
// double check if the finished network reply had a session ID in the header and make
// sure that our session ID matches that value if so
if (networkReply->hasRawHeader(METAVERSE_SESSION_ID_HEADER)) {
_sessionID = networkReply->rawHeader(METAVERSE_SESSION_ID_HEADER);
}
}
});
// There's a cleaner way to fire the JSON/error callbacks below and ensure that deleteLater is called for the
// request reply - unfortunately it requires Qt 5.10 which the Android build does not support as of 06/26/18
if (callbackParams.jsonCallbackReceiver) {
connect(networkReply, &QNetworkReply::finished, callbackParams.jsonCallbackReceiver, [callbackParams, networkReply] {
if (networkReply->error() == QNetworkReply::NoError) {
if (callbackParams.jsonCallbackReceiver && !callbackParams.jsonCallbackMethod.isEmpty()) {
bool invoked = QMetaObject::invokeMethod(callbackParams.jsonCallbackReceiver,
qPrintable(callbackParams.jsonCallbackMethod),
Q_ARG(QNetworkReply*, networkReply));
if (callbackParams.isEmpty()) {
connect(networkReply, &QNetworkReply::finished, networkReply, &QNetworkReply::deleteLater);
} else {
// There's a cleaner way to fire the JSON/error callbacks below and ensure that deleteLater is called for the
// request reply - unfortunately it requires Qt 5.10 which the Android build does not support as of 06/26/18
if (!invoked) {
QString error = "Could not invoke " + callbackParams.jsonCallbackMethod + " with QNetworkReply* "
+ "on errorCallbackReceiver.";
qCWarning(networking) << error;
Q_ASSERT_X(invoked, "AccountManager::passErrorToCallback", qPrintable(error));
}
} else {
if (VERBOSE_HTTP_REQUEST_DEBUGGING) {
qCDebug(networking) << "Received JSON response from metaverse API that has no matching callback.";
qCDebug(networking) << QJsonDocument::fromJson(networkReply->readAll());
}
connect(networkReply, &QNetworkReply::finished, callbackParams.callbackReceiver,
[callbackParams, networkReply] {
if (networkReply->error() == QNetworkReply::NoError) {
if (!callbackParams.jsonCallbackMethod.isEmpty()) {
bool invoked = QMetaObject::invokeMethod(callbackParams.callbackReceiver,
qPrintable(callbackParams.jsonCallbackMethod),
Q_ARG(QNetworkReply*, networkReply));
if (!invoked) {
QString error = "Could not invoke " + callbackParams.jsonCallbackMethod + " with QNetworkReply* "
+ "on callbackReceiver.";
qCWarning(networking) << error;
Q_ASSERT_X(invoked, "AccountManager::passErrorToCallback", qPrintable(error));
}
} else {
if (VERBOSE_HTTP_REQUEST_DEBUGGING) {
qCDebug(networking) << "Received JSON response from metaverse API that has no matching callback.";
qCDebug(networking) << QJsonDocument::fromJson(networkReply->readAll());
}
networkReply->deleteLater();
}
});
}
if (callbackParams.errorCallbackReceiver) {
connect(networkReply, &QNetworkReply::finished, callbackParams.errorCallbackReceiver, [callbackParams, networkReply]{
if (networkReply->error() != QNetworkReply::NoError) {
if (callbackParams.errorCallbackReceiver && !callbackParams.errorCallbackMethod.isEmpty()) {
bool invoked = QMetaObject::invokeMethod(callbackParams.errorCallbackReceiver,
} else {
if (!callbackParams.errorCallbackMethod.isEmpty()) {
bool invoked = QMetaObject::invokeMethod(callbackParams.callbackReceiver,
qPrintable(callbackParams.errorCallbackMethod),
Q_ARG(QNetworkReply*, networkReply));
if (!invoked) {
QString error = "Could not invoke " + callbackParams.errorCallbackMethod + " with QNetworkReply* "
+ "on errorCallbackReceiver.";
+ "on callbackReceiver.";
qCWarning(networking) << error;
Q_ASSERT_X(invoked, "AccountManager::passErrorToCallback", qPrintable(error));
}
@ -382,29 +375,11 @@ void AccountManager::sendRequest(const QString& path,
qCDebug(networking) << networkReply->readAll();
}
}
networkReply->deleteLater();
}
networkReply->deleteLater();
});
}
// double check if the finished network reply had a session ID in the header and make
// sure that our session ID matches that value if so
connect(networkReply, &QNetworkReply::finished, this, [this, callbackParams, networkReply]{
if (networkReply->error() == QNetworkReply::NoError) {
if (networkReply->hasRawHeader(METAVERSE_SESSION_ID_HEADER)) {
_sessionID = networkReply->rawHeader(METAVERSE_SESSION_ID_HEADER);
}
if (!callbackParams.jsonCallbackReceiver) {
networkReply->deleteLater();
}
} else {
if (!callbackParams.errorCallbackReceiver) {
networkReply->deleteLater();
}
}
});
}
}
@ -830,9 +805,8 @@ void AccountManager::processGeneratedKeypair(QByteArray publicKey, QByteArray pr
// setup callback parameters so we know once the keypair upload has succeeded or failed
JSONCallbackParameters callbackParameters;
callbackParameters.jsonCallbackReceiver = this;
callbackParameters.callbackReceiver = this;
callbackParameters.jsonCallbackMethod = "publicKeyUploadSucceeded";
callbackParameters.errorCallbackReceiver = this;
callbackParameters.errorCallbackMethod = "publicKeyUploadFailed";
sendRequest(uploadPath, AccountManagerAuth::Optional, QNetworkAccessManager::PutOperation,

View file

@ -28,18 +28,14 @@
class JSONCallbackParameters {
public:
JSONCallbackParameters(QObject* jsonCallbackReceiver = nullptr, const QString& jsonCallbackMethod = QString(),
QObject* errorCallbackReceiver = nullptr, const QString& errorCallbackMethod = QString(),
QObject* updateReceiver = nullptr, const QString& updateSlot = QString());
JSONCallbackParameters(QObject* callbackReceiver = nullptr, const QString& jsonCallbackMethod = QString(),
const QString& errorCallbackMethod = QString());
bool isEmpty() const { return !jsonCallbackReceiver && !errorCallbackReceiver; }
bool isEmpty() const { return !callbackReceiver; }
QObject* jsonCallbackReceiver;
QObject* callbackReceiver;
QString jsonCallbackMethod;
QObject* errorCallbackReceiver;
QString errorCallbackMethod;
QObject* updateReceiver;
QString updateSlot;
};
namespace AccountManagerAuth {

View file

@ -215,9 +215,8 @@ const JSONCallbackParameters& AddressManager::apiCallbackParameters() {
static JSONCallbackParameters callbackParams;
if (!hasSetupParameters) {
callbackParams.jsonCallbackReceiver = this;
callbackParams.callbackReceiver = this;
callbackParams.jsonCallbackMethod = "handleAPIResponse";
callbackParams.errorCallbackReceiver = this;
callbackParams.errorCallbackMethod = "handleAPIError";
}
@ -917,7 +916,7 @@ void AddressManager::lookupShareableNameForDomainID(const QUuid& domainID) {
// no error callback handling
// in the case of an error we simply assume there is no default place name
callbackParams.jsonCallbackReceiver = this;
callbackParams.callbackReceiver = this;
callbackParams.jsonCallbackMethod = "handleShareableNameAPIResponse";
DependencyManager::get<AccountManager>()->sendRequest(GET_DOMAIN_ID.arg(uuidStringWithoutCurlyBraces(domainID)),

View file

@ -65,7 +65,7 @@ void UserActivityLogger::logAction(QString action, QJsonObject details, JSONCall
// if no callbacks specified, call our owns
if (params.isEmpty()) {
params.errorCallbackReceiver = this;
params.callbackReceiver = this;
params.errorCallbackMethod = "requestError";
}

View file

@ -83,7 +83,7 @@ void Tooltip::requestHyperlinkImage() {
auto accountManager = DependencyManager::get<AccountManager>();
JSONCallbackParameters callbackParams;
callbackParams.jsonCallbackReceiver = this;
callbackParams.callbackReceiver = this;
callbackParams.jsonCallbackMethod = "handleAPIResponse";
accountManager->sendRequest(GET_PLACE.arg(_title),