From 925912ebf552417306a5bd20fd986afda6a582be Mon Sep 17 00:00:00 2001 From: Benjamin C Meyer Date: Tue, 1 Sep 2009 09:51:56 +0200 Subject: QNetworkAccessManager can delete the QAbstractNetworkCache pointer at any point. Rather then keep a separate pointer to the cache in the reply use the pointer kept by the manager so the reply never tries to access a cache pointer that has already been deleted. Autotest: included Merge-request: 1124 Reviewed-by: Thiago Macieira --- src/network/access/qnetworkaccessbackend.cpp | 4 ++- src/network/access/qnetworkaccessmanager.cpp | 3 -- src/network/access/qnetworkreplyimpl.cpp | 35 ++++++++++++---------- src/network/access/qnetworkreplyimpl_p.h | 3 +- .../tst_qabstractnetworkcache.cpp | 16 ++++++++++ 5 files changed, 40 insertions(+), 21 deletions(-) diff --git a/src/network/access/qnetworkaccessbackend.cpp b/src/network/access/qnetworkaccessbackend.cpp index 615ce7c..947313c 100644 --- a/src/network/access/qnetworkaccessbackend.cpp +++ b/src/network/access/qnetworkaccessbackend.cpp @@ -176,7 +176,9 @@ QList QNetworkAccessBackend::proxyList() const QAbstractNetworkCache *QNetworkAccessBackend::networkCache() const { - return reply->networkCache; // should be the same as manager->networkCache + if (!manager) + return 0; + return manager->networkCache; } void QNetworkAccessBackend::setCachingEnabled(bool enable) diff --git a/src/network/access/qnetworkaccessmanager.cpp b/src/network/access/qnetworkaccessmanager.cpp index aafe44b..5a8756b 100644 --- a/src/network/access/qnetworkaccessmanager.cpp +++ b/src/network/access/qnetworkaccessmanager.cpp @@ -692,9 +692,6 @@ QNetworkReply *QNetworkAccessManager::createRequest(QNetworkAccessManager::Opera // third step: setup the reply priv->setup(op, request, outgoingData); - if (request.attribute(QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::PreferNetwork).toInt() != - QNetworkRequest::AlwaysNetwork) - priv->setNetworkCache(d->networkCache); #ifndef QT_NO_NETWORKPROXY QList proxyList = d->queryProxy(QNetworkProxyQuery(request.url())); priv->proxyList = proxyList; diff --git a/src/network/access/qnetworkreplyimpl.cpp b/src/network/access/qnetworkreplyimpl.cpp index c37c66d..c805d59 100644 --- a/src/network/access/qnetworkreplyimpl.cpp +++ b/src/network/access/qnetworkreplyimpl.cpp @@ -53,7 +53,7 @@ QT_BEGIN_NAMESPACE inline QNetworkReplyImplPrivate::QNetworkReplyImplPrivate() : backend(0), outgoingData(0), - copyDevice(0), networkCache(0), + copyDevice(0), cacheEnabled(false), cacheSaveDevice(0), notificationHandlingPaused(false), bytesDownloaded(0), lastBytesDownloaded(-1), bytesUploaded(-1), @@ -203,11 +203,6 @@ void QNetworkReplyImplPrivate::setup(QNetworkAccessManager::Operation op, const QMetaObject::invokeMethod(q, "_q_startOperation", Qt::QueuedConnection); } -void QNetworkReplyImplPrivate::setNetworkCache(QAbstractNetworkCache *nc) -{ - networkCache = nc; -} - void QNetworkReplyImplPrivate::backendNotify(InternalNotifications notification) { Q_Q(QNetworkReplyImpl); @@ -277,17 +272,27 @@ void QNetworkReplyImplPrivate::resumeNotificationHandling() QCoreApplication::postEvent(q, new QEvent(QEvent::NetworkReplyUpdated)); } +QAbstractNetworkCache *QNetworkReplyImplPrivate::networkCache() const +{ + if (!backend) + return 0; + return backend->networkCache(); +} + void QNetworkReplyImplPrivate::createCache() { // check if we can save and if we're allowed to - if (!networkCache || !request.attribute(QNetworkRequest::CacheSaveControlAttribute, true).toBool()) + if (!networkCache() + || request.attribute(QNetworkRequest::CacheLoadControlAttribute, + QNetworkRequest::PreferNetwork).toInt() + == QNetworkRequest::AlwaysNetwork) return; cacheEnabled = true; } bool QNetworkReplyImplPrivate::isCachingEnabled() const { - return (cacheEnabled && networkCache != 0); + return (cacheEnabled && networkCache() != 0); } void QNetworkReplyImplPrivate::setCachingEnabled(bool enable) @@ -311,7 +316,7 @@ void QNetworkReplyImplPrivate::setCachingEnabled(bool enable) qDebug("QNetworkReplyImpl: setCachingEnabled(true) called after setCachingEnabled(false) -- " "backend %s probably needs to be fixed", backend->metaObject()->className()); - networkCache->remove(url); + networkCache()->remove(url); cacheSaveDevice = 0; cacheEnabled = false; } @@ -320,9 +325,9 @@ void QNetworkReplyImplPrivate::setCachingEnabled(bool enable) void QNetworkReplyImplPrivate::completeCacheSave() { if (cacheEnabled && errorCode != QNetworkReplyImpl::NoError) { - networkCache->remove(url); + networkCache()->remove(url); } else if (cacheEnabled && cacheSaveDevice) { - networkCache->insert(cacheSaveDevice); + networkCache()->insert(cacheSaveDevice); } cacheSaveDevice = 0; cacheEnabled = false; @@ -385,15 +390,15 @@ void QNetworkReplyImplPrivate::feed(const QByteArray &data) metaData.setAttributes(attributes); } - cacheSaveDevice = networkCache->prepare(metaData); + cacheSaveDevice = networkCache()->prepare(metaData); if (!cacheSaveDevice || (cacheSaveDevice && !cacheSaveDevice->isOpen())) { if (cacheSaveDevice && !cacheSaveDevice->isOpen()) qCritical("QNetworkReplyImpl: network cache returned a device that is not open -- " "class %s probably needs to be fixed", - networkCache->metaObject()->className()); + networkCache()->metaObject()->className()); - networkCache->remove(url); + networkCache()->remove(url); cacheSaveDevice = 0; cacheEnabled = false; } @@ -524,7 +529,7 @@ QNetworkReplyImpl::~QNetworkReplyImpl() { Q_D(QNetworkReplyImpl); if (d->isCachingEnabled()) - d->networkCache->remove(url()); + d->networkCache()->remove(url()); } void QNetworkReplyImpl::abort() diff --git a/src/network/access/qnetworkreplyimpl_p.h b/src/network/access/qnetworkreplyimpl_p.h index 0accaab..f723b7f 100644 --- a/src/network/access/qnetworkreplyimpl_p.h +++ b/src/network/access/qnetworkreplyimpl_p.h @@ -128,7 +128,6 @@ public: void setup(QNetworkAccessManager::Operation op, const QNetworkRequest &request, QIODevice *outgoingData); - void setNetworkCache(QAbstractNetworkCache *networkCache); void pauseNotificationHandling(); void resumeNotificationHandling(); @@ -153,7 +152,7 @@ public: QNetworkAccessBackend *backend; QIODevice *outgoingData; QIODevice *copyDevice; - QAbstractNetworkCache *networkCache; + QAbstractNetworkCache *networkCache() const; bool cacheEnabled; QIODevice *cacheSaveDevice; diff --git a/tests/auto/qabstractnetworkcache/tst_qabstractnetworkcache.cpp b/tests/auto/qabstractnetworkcache/tst_qabstractnetworkcache.cpp index e338075..ee49210 100644 --- a/tests/auto/qabstractnetworkcache/tst_qabstractnetworkcache.cpp +++ b/tests/auto/qabstractnetworkcache/tst_qabstractnetworkcache.cpp @@ -69,6 +69,8 @@ private slots: void cacheControl_data(); void cacheControl(); + void deleteCache(); + private: void check(); }; @@ -269,6 +271,20 @@ void tst_QAbstractNetworkCache::check() QCOMPARE(diskCache->gotData, fetchFromCache); } +void tst_QAbstractNetworkCache::deleteCache() +{ + QNetworkAccessManager manager; + NetworkDiskCache *diskCache = new NetworkDiskCache(&manager); + manager.setCache(diskCache); + + QString url = "httpcachetest_cachecontrol.cgi?max-age=1000"; + QNetworkRequest request(QUrl(TESTFILE + url)); + QNetworkReply *reply = manager.get(request); + QSignalSpy downloaded1(reply, SIGNAL(finished())); + manager.setCache(0); + QTRY_COMPARE(downloaded1.count(), 1); +} + QTEST_MAIN(tst_QAbstractNetworkCache) #include "tst_qabstractnetworkcache.moc" -- cgit v0.12