diff --git a/src/network/access/qhttpthreaddelegate.cpp b/src/network/access/qhttpthreaddelegate.cpp index 8db56222d2..50a14b6258 100644 --- a/src/network/access/qhttpthreaddelegate.cpp +++ b/src/network/access/qhttpthreaddelegate.cpp @@ -226,6 +226,7 @@ QHttpThreadDelegate::QHttpThreadDelegate(QObject *parent) : , pendingDownloadData() , pendingDownloadProgress() , synchronous(false) + , connectionCacheExpiryTimeoutSeconds(-1) , incomingStatusCode(0) , isPipeliningUsed(false) , isHttp2Used(false) @@ -344,7 +345,7 @@ void QHttpThreadDelegate::startRequest() #endif httpConnection->setPeerVerifyName(httpRequest.peerVerifyName()); // cache the QHttpNetworkConnection corresponding to this cache key - connections.localData()->addEntry(cacheKey, httpConnection); + connections.localData()->addEntry(cacheKey, httpConnection, connectionCacheExpiryTimeoutSeconds); } else { if (httpRequest.withCredentials()) { QNetworkAuthenticationCredential credential = authenticationManager->fetchCachedCredentials(httpRequest.url(), nullptr); diff --git a/src/network/access/qhttpthreaddelegate_p.h b/src/network/access/qhttpthreaddelegate_p.h index f919b403f9..f0b9b8edf3 100644 --- a/src/network/access/qhttpthreaddelegate_p.h +++ b/src/network/access/qhttpthreaddelegate_p.h @@ -105,6 +105,7 @@ public: #endif QSharedPointer authenticationManager; bool synchronous; + qint64 connectionCacheExpiryTimeoutSeconds; // outgoing, Retrieved in the synchronous HTTP case QByteArray synchronousDownloadData; diff --git a/src/network/access/qnetworkaccesscache.cpp b/src/network/access/qnetworkaccesscache.cpp index 4d65761a0b..7c8da551ad 100644 --- a/src/network/access/qnetworkaccesscache.cpp +++ b/src/network/access/qnetworkaccesscache.cpp @@ -148,18 +148,46 @@ void QNetworkAccessCache::linkEntry(const QByteArray &key) Q_ASSERT(node->older == nullptr && node->newer == nullptr); Q_ASSERT(node->useCount == 0); + + node->timestamp = QDateTime::currentDateTimeUtc().addSecs(node->object->expiryTimeoutSeconds); +#ifdef QT_DEBUG + qDebug() << "QNetworkAccessCache case trying to insert=" <timestamp; + Node *current = newest; + while (current) { + qDebug() << "QNetworkAccessCache item=" << QString::fromUtf8(current->key) << current->timestamp << (current==newest? "newest":"") << (current==oldest? "oldest":""); + current = current->older; + } +#endif + if (newest) { Q_ASSERT(newest->newer == nullptr); - newest->newer = node; - node->older = newest; + if (newest->timestamp < node->timestamp) { + // Insert as new newest. + node->older = newest; + newest->newer = node; + newest = node; + Q_ASSERT(newest->newer == nullptr); + } else { + // Insert in a sorted way, as different nodes might have had different expiryTimeoutSeconds set. + Node *current = newest; + while (current->older != nullptr && current->older->timestamp >= node->timestamp) { + current = current->older; + } + node->older = current->older; + current->older = node; + if (node->older == nullptr) { + oldest = node; + Q_ASSERT(oldest->older == nullptr); + } + } + } else { + // no newest yet + newest = node; } if (!oldest) { // there are no entries, so this is the oldest one too oldest = node; } - - node->timestamp = QDateTime::currentDateTimeUtc().addSecs(ExpiryTime); - newest = node; } /*! @@ -195,15 +223,16 @@ void QNetworkAccessCache::updateTimer() if (!oldest) return; - int interval = QDateTime::currentDateTimeUtc().secsTo(oldest->timestamp); + qint64 interval = QDateTime::currentDateTimeUtc().msecsTo(oldest->timestamp); if (interval <= 0) { interval = 0; - } else { - // round up the interval - interval = (interval + 15) & ~16; } - timer.start(interval * 1000, this); + // Plus 10 msec so we don't spam timer events if date comparisons are too fuzzy. + // This code used to do (broken) rounding, but for ConnectionCacheExpiryTimeoutSecondsAttribute + // to work we cannot do this. + // See discussion in https://codereview.qt-project.org/c/qt/qtbase/+/337464 + timer.start(interval + 10, this); } bool QNetworkAccessCache::emitEntryReady(Node *node, QObject *target, const char *member) @@ -226,7 +255,6 @@ void QNetworkAccessCache::timerEvent(QTimerEvent *) while (oldest && oldest->timestamp < now) { Node *next = oldest->newer; oldest->object->dispose(); - hash.remove(oldest->key); // oldest gets deleted delete oldest; oldest = next; @@ -241,7 +269,7 @@ void QNetworkAccessCache::timerEvent(QTimerEvent *) updateTimer(); } -void QNetworkAccessCache::addEntry(const QByteArray &key, CacheableObject *entry) +void QNetworkAccessCache::addEntry(const QByteArray &key, CacheableObject *entry, qint64 connectionCacheExpiryTimeoutSeconds) { Q_ASSERT(!key.isEmpty()); @@ -260,8 +288,15 @@ void QNetworkAccessCache::addEntry(const QByteArray &key, CacheableObject *entry node->object->dispose(); node->object = entry; node->object->key = key; + if (connectionCacheExpiryTimeoutSeconds > -1) { + node->object->expiryTimeoutSeconds = connectionCacheExpiryTimeoutSeconds; // via ConnectionCacheExpiryTimeoutSecondsAttribute + } else { + node->object->expiryTimeoutSeconds = ExpiryTime; + } node->key = key; node->useCount = 1; + + // It gets only put into the expiry list in linkEntry (from releaseEntry), when it is not used anymore. } bool QNetworkAccessCache::hasEntry(const QByteArray &key) const diff --git a/src/network/access/qnetworkaccesscache_p.h b/src/network/access/qnetworkaccesscache_p.h index 9f7001d044..e31722e6fd 100644 --- a/src/network/access/qnetworkaccesscache_p.h +++ b/src/network/access/qnetworkaccesscache_p.h @@ -79,6 +79,7 @@ public: QByteArray key; bool expires; bool shareable; + qint64 expiryTimeoutSeconds; public: CacheableObject(); virtual ~CacheableObject(); @@ -95,7 +96,7 @@ public: void clear(); - void addEntry(const QByteArray &key, CacheableObject *entry); + void addEntry(const QByteArray &key, CacheableObject *entry, qint64 connectionCacheExpiryTimeoutSeconds = -1); bool hasEntry(const QByteArray &key) const; bool requestEntry(const QByteArray &key, QObject *target, const char *member); CacheableObject *requestEntryNow(const QByteArray &key); diff --git a/src/network/access/qnetworkreplyhttpimpl.cpp b/src/network/access/qnetworkreplyhttpimpl.cpp index 69597b1ec8..4fc1f81c95 100644 --- a/src/network/access/qnetworkreplyhttpimpl.cpp +++ b/src/network/access/qnetworkreplyhttpimpl.cpp @@ -812,6 +812,9 @@ void QNetworkReplyHttpImplPrivate::postRequest(const QNetworkRequest &newHttpReq // Propagate Http/2 settings: delegate->http2Parameters = request.http2Configuration(); + if (request.attribute(QNetworkRequest::ConnectionCacheExpiryTimeoutSecondsAttribute).isValid()) + delegate->connectionCacheExpiryTimeoutSeconds = request.attribute(QNetworkRequest::ConnectionCacheExpiryTimeoutSecondsAttribute).toInt(); + // For the synchronous HTTP, this is the normal way the delegate gets deleted // For the asynchronous HTTP this is a safety measure, the delegate deletes itself when HTTP is finished QMetaObject::Connection threadFinishedConnection = diff --git a/src/network/access/qnetworkrequest.cpp b/src/network/access/qnetworkrequest.cpp index f6a6f09670..688c29935c 100644 --- a/src/network/access/qnetworkrequest.cpp +++ b/src/network/access/qnetworkrequest.cpp @@ -319,6 +319,12 @@ QT_BEGIN_NAMESPACE the QNetworkReply after having emitted "finished". (This value was introduced in 5.14.) + \value ConnectionCacheExpiryTimeoutSecondsAttribute + Requests only, type: QMetaType::Int + To set when the TCP connections to a server (HTTP1 and HTTP2) should + be closed after the last pending request had been processed. + (This value was introduced in 6.3.) + \value User Special type. Additional information can be passed in QVariants with types ranging from User to UserMax. The default diff --git a/src/network/access/qnetworkrequest.h b/src/network/access/qnetworkrequest.h index 72848ff490..bdcb1c80a9 100644 --- a/src/network/access/qnetworkrequest.h +++ b/src/network/access/qnetworkrequest.h @@ -97,6 +97,7 @@ public: Http2DirectAttribute, ResourceTypeAttribute, // internal AutoDeleteReplyOnFinishAttribute, + ConnectionCacheExpiryTimeoutSecondsAttribute, User = 1000, UserMax = 32767 diff --git a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp index 261e738955..a824a15b49 100644 --- a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp +++ b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp @@ -452,6 +452,8 @@ private Q_SLOTS: void httpAbort(); + void closeClientSideConnectionEagerlyQtbug20726(); + void dontInsertPartialContentIntoTheCache(); void httpUserAgent(); @@ -7936,6 +7938,56 @@ void tst_QNetworkReply::httpAbort() QCOMPARE(reply3->error(), QNetworkReply::NoError); } +void tst_QNetworkReply::closeClientSideConnectionEagerlyQtbug20726() +{ + QNetworkAccessManager manager; // function local instance + // Setup HTTP servers + MiniHttpServer server("HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: Keep-Alive\r\n\r\n", false); + server.doClose = false; // server should not disconnect. + + MiniHttpServer serverNotEagerClientClose("HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: Keep-Alive\r\n\r\n", false); + serverNotEagerClientClose.doClose = false; // server should not disconnect. + QUrl urlNotEager(QLatin1String("http://localhost")); + urlNotEager.setPort(serverNotEagerClientClose.serverPort()); + QNetworkRequest requestNotEager(urlNotEager); + QNetworkReplyPtr replyNotEager(manager.get(requestNotEager)); + QCOMPARE(waitForFinish(replyNotEager), Success); + // The reply was finished, the connection should be hanging and waiting to be expired + + // Another server not eager to close, the connection should be hanging and waiting to be expired + MiniHttpServer serverNotEagerClientClose2("HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: Keep-Alive\r\n\r\n", false); + serverNotEagerClientClose2.doClose = false; // server should not disconnect. + QUrl urlNotEager2(QLatin1String("http://localhost")); + urlNotEager2.setPort(serverNotEagerClientClose2.serverPort()); + QNetworkRequest requestNotEager2(urlNotEager2); + QNetworkReplyPtr replyNotEager2(manager.get(requestNotEager2)); + QCOMPARE(waitForFinish(replyNotEager2), Success); + + // However for this one we want to eagerly close. + QUrl url(QLatin1String("http://localhost")); + url.setPort(server.serverPort()); + QNetworkRequest request(url); + request.setAttribute(QNetworkRequest::ConnectionCacheExpiryTimeoutSecondsAttribute, 0); + QNetworkReplyPtr reply(manager.get(request)); + qDebug() << reply->request().url() << replyNotEager->request().url(); + QCOMPARE(waitForFinish(reply), Success); + QCOMPARE(reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(), 200); + // Socket from server to QNAM still connected? + QVERIFY (!server.client.isNull()); + QVERIFY (server.client->state() == QTcpSocket::ConnectedState); + // Wait a bit + QTest::qWait(1*1000); + // The QNAM should have disconnected the socket, so on our server it's disconnected now. + QVERIFY (!server.client.isNull()); + QVERIFY (server.client->state() != QTcpSocket::ConnectedState); + + // Now we check the not eager reply, it should still be connected. + QCOMPARE(replyNotEager->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(), 200); + QCOMPARE(serverNotEagerClientClose.client->state(), QTcpSocket::ConnectedState); + QCOMPARE(replyNotEager2->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(), 200); + QCOMPARE(serverNotEagerClientClose2.client->state(), QTcpSocket::ConnectedState); +} + void tst_QNetworkReply::dontInsertPartialContentIntoTheCache() { QByteArray reply206 =