network internals: do not try to cache a deleted entry

We were keeping a dangling pointer to a non-existent QIODevice around
which would lead to a crash.

This is not reproducible in Qt5 anymore.

Task-number: QTBUG-17400
Change-Id: I19af701a42e48c05d04dec18eca9f1bfc7e1f4bb
Reviewed-by: Richard J. Moore <rich@kde.org>

3 files changed, 52 insertions, 0 deletions

diff --git a/src/network/access/qnetworkreplyimpl.cpp b/src/network/access/qnetworkreplyimpl.cpp
index 04956580e9..7ee7386b66 100644
--- a/src/network/access/qnetworkreplyimpl.cpp
+++ b/src/network/access/qnetworkreplyimpl.cpp
@@ -505,6 +505,13 @@ void QNetworkReplyImplPrivate::_q_cacheDestroyed()
     cacheEnabled = false;
 }
 
+void QNetworkReplyImplPrivate::_q_cacheSaveDeviceAboutToClose()
+{
+    // do not keep a dangling pointer to the device around (device
+    // is closing because e.g. QAbstractNetworkCache::remove() was called).
+    cacheSaveDevice = 0;
+}
+
 void QNetworkReplyImplPrivate::completeCacheSave()
 {
     Q_Q(QNetworkReplyImpl);
@@ -565,6 +572,9 @@ void QNetworkReplyImplPrivate::initCacheSaveDevice()
 
     cacheSaveDevice = networkCache()->prepare(metaData);
 
+    if (cacheSaveDevice)
+        q->connect(cacheSaveDevice, SIGNAL(aboutToClose()), SLOT(_q_cacheSaveDeviceAboutToClose()));
+
     if (!cacheSaveDevice || (cacheSaveDevice && !cacheSaveDevice->isOpen())) {
         if (cacheSaveDevice && !cacheSaveDevice->isOpen())
             qCritical("QNetworkReplyImpl: network cache returned a device that is not open -- "
diff --git a/src/network/access/qnetworkreplyimpl_p.h b/src/network/access/qnetworkreplyimpl_p.h
index 3947e7982e..d59e6a0ab6 100644
--- a/src/network/access/qnetworkreplyimpl_p.h
+++ b/src/network/access/qnetworkreplyimpl_p.h
@@ -105,6 +105,7 @@ public:
     Q_PRIVATE_SLOT(d_func(), void _q_networkSessionFailed())
 #endif
     Q_PRIVATE_SLOT(d_func(), void _q_cacheDestroyed())
+    Q_PRIVATE_SLOT(d_func(), void _q_cacheSaveDeviceAboutToClose())
 };
 
 class QNetworkReplyImplPrivate: public QNetworkReplyPrivate
@@ -142,6 +143,7 @@ public:
     void _q_networkSessionFailed();
 #endif
     void _q_cacheDestroyed();
+    void _q_cacheSaveDeviceAboutToClose();
 
     void setup(QNetworkAccessManager::Operation op, const QNetworkRequest &request,
                QIODevice *outgoingData);
diff --git a/tests/auto/qnetworkdiskcache/tst_qnetworkdiskcache.cpp b/tests/auto/qnetworkdiskcache/tst_qnetworkdiskcache.cpp
index 2bb967c3d3..fabec7823a 100644
--- a/tests/auto/qnetworkdiskcache/tst_qnetworkdiskcache.cpp
+++ b/tests/auto/qnetworkdiskcache/tst_qnetworkdiskcache.cpp
@@ -58,6 +58,7 @@ public slots:
     void cleanupTestCase();
     void init();
     void cleanup();
+    void accessAfterRemoveReadyReadSlot();
 
 private slots:
     void qnetworkdiskcache_data();
@@ -70,6 +71,7 @@ private slots:
     void data();
     void metaData();
     void remove();
+    void accessAfterRemove(); // QTBUG-17400
     void setCacheDirectory_data();
     void setCacheDirectory();
     void updateMetaData();
@@ -82,6 +84,10 @@ private slots:
     void sync();
 
     void crashWhenParentingCache();
+
+private:
+    QUrl url; // used by accessAfterRemove()
+    QNetworkDiskCache *diskCache; // used by accessAfterRemove()
 };
 
 // FIXME same as in tst_qnetworkreply.cpp .. could be unified
@@ -360,6 +366,40 @@ void tst_QNetworkDiskCache::remove()
     QCOMPARE(countFiles(cacheDirectory).count(), NUM_SUBDIRECTORIES + 2);
 }
 
+void tst_QNetworkDiskCache::accessAfterRemove() // QTBUG-17400
+{
+    QByteArray data("HTTP/1.1 200 OK\r\n"
+                    "Content-Length: 1\r\n"
+                    "\r\n"
+                    "a");
+
+    MiniHttpServer server(data);
+
+    QNetworkAccessManager *manager = new QNetworkAccessManager();
+    SubQNetworkDiskCache subCache;
+    subCache.setCacheDirectory(QLatin1String("cacheDir"));
+    diskCache = &subCache;
+    manager->setCache(&subCache);
+
+    url = QUrl("http://127.0.0.1:" + QString::number(server.serverPort()));
+    QNetworkRequest request(url);
+
+    QNetworkReply *reply = manager->get(request);
+    connect(reply, SIGNAL(readyRead()), this, SLOT(accessAfterRemoveReadyReadSlot()));
+    connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop()));
+
+    QTestEventLoop::instance().enterLoop(5);
+    QVERIFY(!QTestEventLoop::instance().timeout());
+
+    reply->deleteLater();
+    manager->deleteLater();
+}
+
+void tst_QNetworkDiskCache::accessAfterRemoveReadyReadSlot()
+{
+    diskCache->remove(url); // this used to cause a crash later on
+}
+
 void tst_QNetworkDiskCache::setCacheDirectory_data()
 {
     QTest::addColumn<QString>("cacheDir");