Account/Credentials: Have identical lifetimes

The QNAM may continue to outlive both.

Rename Credentials::getQNAM() to createQNAM() while we're at it - it's
used to make a new QNAM that will subsequently be owned by the Account
object.

See d01065b9a12e69ca493a232f3a8e8f3d416fed52 for rationale.

Relates to
d40c56eda561e3a541bf1b23f70fa8d659d3037e
147cf798a6f13c9b53a9f1fb2db1ef26c8c63273

diff --git a/src/gui/creds/shibbolethcredentials.cpp b/src/gui/creds/shibbolethcredentials.cpp
index cc3d6fb686dc63384807f334eb167d9a20328438..b7fca6dd3d5043deb129d20d511c02854208a9a8 100644 (file)
--- a/src/gui/creds/shibbolethcredentials.cpp
+++ b/src/gui/creds/shibbolethcredentials.cpp
@@ -90,7 +90,7 @@ QString ShibbolethCredentials::user() const
     return _user;
 }
 
-QNetworkAccessManager *ShibbolethCredentials::getQNAM() const
+QNetworkAccessManager *ShibbolethCredentials::createQNAM() const
 {
     QNetworkAccessManager *qnam(new AccessManager);
     connect(qnam, SIGNAL(finished(QNetworkReply *)),

diff --git a/src/gui/creds/shibbolethcredentials.h b/src/gui/creds/shibbolethcredentials.h
index 46b6bc08dfd2b4fc91a57fcca47a42ddc978300e..a4909ed7412df24a5d57f51edb5718cf94471ad8 100644 (file)
--- a/src/gui/creds/shibbolethcredentials.h
+++ b/src/gui/creds/shibbolethcredentials.h
@@ -53,7 +53,7 @@ public:
     void setAccount(Account *account) Q_DECL_OVERRIDE;
     QString authType() const Q_DECL_OVERRIDE;
     QString user() const Q_DECL_OVERRIDE;
-    QNetworkAccessManager *getQNAM() const Q_DECL_OVERRIDE;
+    QNetworkAccessManager *createQNAM() const Q_DECL_OVERRIDE;
     bool ready() const Q_DECL_OVERRIDE;
     void fetchFromKeychain() Q_DECL_OVERRIDE;
     void askFromUser() Q_DECL_OVERRIDE;

diff --git a/src/libsync/account.cpp b/src/libsync/account.cpp
index ee83e3deecb98d14303aff347d45fc491fc0542d..45168033f647c5679001874025e90b96c62632d1 100644 (file)
--- a/src/libsync/account.cpp
+++ b/src/libsync/account.cpp
@@ -136,11 +136,13 @@ void Account::setCredentials(AbstractCredentials *cred)
 
     // The order for these two is important! Reading the credential's
     // settings accesses the account as well as account->_credentials,
-    // so deleteLater must be used.
-    _credentials = QSharedPointer<AbstractCredentials>(cred, &QObject::deleteLater);
+    _credentials.reset(cred);
     cred->setAccount(this);
 
-    _am = QSharedPointer<QNetworkAccessManager>(_credentials->getQNAM(), &QObject::deleteLater);
+    // Note: This way the QNAM can outlive the Account and Credentials.
+    // This is necessary to avoid issues with the QNAM being deleted while
+    // processing slotHandleSslErrors().
+    _am = QSharedPointer<QNetworkAccessManager>(_credentials->createQNAM(), &QObject::deleteLater);
 
     if (jar) {
         _am->setCookieJar(jar);
@@ -205,7 +207,7 @@ void Account::resetNetworkAccessManager()
 
     // Use a QSharedPointer to allow locking the life of the QNAM on the stack.
     // Make it call deleteLater to make sure that we can return to any QNAM stack frames safely.
-    _am = QSharedPointer<QNetworkAccessManager>(_credentials->getQNAM(), &QObject::deleteLater);
+    _am = QSharedPointer<QNetworkAccessManager>(_credentials->createQNAM(), &QObject::deleteLater);
 
     _am->setCookieJar(jar); // takes ownership of the old cookie jar
     connect(_am.data(), SIGNAL(sslErrors(QNetworkReply *, QList<QSslError>)),

diff --git a/src/libsync/account.h b/src/libsync/account.h
index d48b27b15c3620cda52bc64b8702db59d7363bf5..b34342d4525b4180de42b3deb5bb219f7b5f667e 100644 (file)
--- a/src/libsync/account.h
+++ b/src/libsync/account.h
@@ -246,7 +246,7 @@ private:
     QScopedPointer<AbstractSslErrorHandler> _sslErrorHandler;
     QuotaInfo *_quotaInfo;
     QSharedPointer<QNetworkAccessManager> _am;
-    QSharedPointer<AbstractCredentials> _credentials;
+    QScopedPointer<AbstractCredentials> _credentials;
 
     /// Certificates that were explicitly rejected by the user
     QList<QSslCertificate> _rejectedCertificates;

diff --git a/src/libsync/creds/abstractcredentials.h b/src/libsync/creds/abstractcredentials.h
index 09ba50e3e453688485bc3c193b94beab2a9cc474..9958b56666ac8b5f34ba549d629b722198a35019 100644 (file)
--- a/src/libsync/creds/abstractcredentials.h
+++ b/src/libsync/creds/abstractcredentials.h
@@ -43,7 +43,7 @@ public:
 
     virtual QString authType() const = 0;
     virtual QString user() const = 0;
-    virtual QNetworkAccessManager *getQNAM() const = 0;
+    virtual QNetworkAccessManager *createQNAM() const = 0;
 
     /** Whether there are credentials that can be used for a connection attempt. */
     virtual bool ready() const = 0;

diff --git a/src/libsync/creds/dummycredentials.cpp b/src/libsync/creds/dummycredentials.cpp
index c55e979ac12baa2c5a7041f09b03b4aa9a81967c..3c5609d356787eb1a7634d1e0b86a57cba08738d 100644 (file)
--- a/src/libsync/creds/dummycredentials.cpp
+++ b/src/libsync/creds/dummycredentials.cpp
@@ -27,7 +27,7 @@ QString DummyCredentials::user() const
     return _user;
 }
 
-QNetworkAccessManager *DummyCredentials::getQNAM() const
+QNetworkAccessManager *DummyCredentials::createQNAM() const
 {
     return new AccessManager;
 }

diff --git a/src/libsync/creds/dummycredentials.h b/src/libsync/creds/dummycredentials.h
index ad56a08dd5093419f5390f8ec9eb7875a6726ef3..6729b2e5a5156c8867dc2ed0943c9c922447b66a 100644 (file)
--- a/src/libsync/creds/dummycredentials.h
+++ b/src/libsync/creds/dummycredentials.h
@@ -28,7 +28,7 @@ public:
     QString _password;
     QString authType() const Q_DECL_OVERRIDE;
     QString user() const Q_DECL_OVERRIDE;
-    QNetworkAccessManager *getQNAM() const Q_DECL_OVERRIDE;
+    QNetworkAccessManager *createQNAM() const Q_DECL_OVERRIDE;
     bool ready() const Q_DECL_OVERRIDE;
     bool stillValid(QNetworkReply *reply) Q_DECL_OVERRIDE;
     void fetchFromKeychain() Q_DECL_OVERRIDE;

diff --git a/src/libsync/creds/httpcredentials.cpp b/src/libsync/creds/httpcredentials.cpp
index 1670e0101e4a576cc6678cf66b8e9026b55e3c85..bd1cc7aef75a1ae3967860b83e67ee4398b9a8e0 100644 (file)
--- a/src/libsync/creds/httpcredentials.cpp
+++ b/src/libsync/creds/httpcredentials.cpp
@@ -58,7 +58,7 @@ protected:
     QNetworkReply *createRequest(Operation op, const QNetworkRequest &request, QIODevice *outgoingData) Q_DECL_OVERRIDE
     {
         QNetworkRequest req(request);
-        if (!_cred->password().isEmpty()) {
+        if (_cred && !_cred->password().isEmpty()) {
             if (_cred->isUsingOAuth()) {
                 req.setRawHeader("Authorization", "Bearer " + _cred->password().toUtf8());
             } else {
@@ -72,7 +72,7 @@ protected:
             req.setRawHeader("Authorization", "Basic " + credHash);
         }
 
-        if (!_cred->_clientSslKey.isNull() && !_cred->_clientSslCertificate.isNull()) {
+        if (_cred && !_cred->_clientSslKey.isNull() && !_cred->_clientSslCertificate.isNull()) {
             // SSL configuration
             QSslConfiguration sslConfiguration = req.sslConfiguration();
             sslConfiguration.setLocalCertificate(_cred->_clientSslCertificate);
@@ -85,7 +85,9 @@ protected:
     }
 
 private:
-    const HttpCredentials *_cred;
+    // The credentials object dies along with the account, while the QNAM might
+    // outlive both.
+    QPointer<const HttpCredentials> _cred;
 };
 
 
@@ -135,7 +137,7 @@ void HttpCredentials::setAccount(Account *account)
     }
 }
 
-QNetworkAccessManager *HttpCredentials::getQNAM() const
+QNetworkAccessManager *HttpCredentials::createQNAM() const
 {
     AccessManager *qnam = new HttpCredentialsAccessManager(this);
 

diff --git a/src/libsync/creds/httpcredentials.h b/src/libsync/creds/httpcredentials.h
index 03f3b8c702b0a496fbb8d8cdc1d641ffbdc2093c..85e6d092af1cf5bff4c565c6177408e8615f91da 100644 (file)
--- a/src/libsync/creds/httpcredentials.h
+++ b/src/libsync/creds/httpcredentials.h
@@ -79,7 +79,7 @@ public:
     HttpCredentials(const QString &user, const QString &password, const QSslCertificate &certificate = QSslCertificate(), const QSslKey &key = QSslKey());
 
     QString authType() const Q_DECL_OVERRIDE;
-    QNetworkAccessManager *getQNAM() const Q_DECL_OVERRIDE;
+    QNetworkAccessManager *createQNAM() const Q_DECL_OVERRIDE;
     bool ready() const Q_DECL_OVERRIDE;
     void fetchFromKeychain() Q_DECL_OVERRIDE;
     bool stillValid(QNetworkReply *reply) Q_DECL_OVERRIDE;

diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h
index 997e5054e5419e2e1f9f671880e3db83f5c25326..b90ff480b1d3ba11620cac028ea52b36a2dd5fd4 100644 (file)
--- a/test/syncenginetestutils.h
+++ b/test/syncenginetestutils.h
@@ -778,7 +778,7 @@ public:
     FakeCredentials(QNetworkAccessManager *qnam) : _qnam{qnam} { }
     virtual QString authType() const { return "test"; }
     virtual QString user() const { return "admin"; }
-    virtual QNetworkAccessManager* getQNAM() const { return _qnam; }
+    virtual QNetworkAccessManager *createQNAM() const { return _qnam; }
     virtual bool ready() const { return true; }
     virtual void fetchFromKeychain() { }
     virtual void askFromUser() { }