From 7a2bfae5e4d1299909efa43238c6a7dd0ab3bf5c Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Thu, 20 Jun 2024 12:13:27 +0800 Subject: [PATCH] Improve connection handling 1. Previously unhandled connections will stay in pending state. It won't be closed until timeout happened. This may lead to wasting system resources. Now the (over-limit) connection is actively rejected. 2. When out-of-memory occurs here, reject the new connection instead of throwing exception and crash. 3. Also clean up some unused bits. PR #20961. --- .../bittorrent/bencoderesumedatastorage.cpp | 1 - src/base/http/connection.cpp | 11 +---- src/base/http/connection.h | 5 +- src/base/http/server.cpp | 48 +++++++++++-------- src/gui/transferlistfilterswidget.cpp | 1 - 5 files changed, 32 insertions(+), 34 deletions(-) diff --git a/src/base/bittorrent/bencoderesumedatastorage.cpp b/src/base/bittorrent/bencoderesumedatastorage.cpp index 4107c5aa9..a34e5a34a 100644 --- a/src/base/bittorrent/bencoderesumedatastorage.cpp +++ b/src/base/bittorrent/bencoderesumedatastorage.cpp @@ -40,7 +40,6 @@ #include #include -#include "base/algorithm.h" #include "base/exceptions.h" #include "base/global.h" #include "base/logger.h" diff --git a/src/base/http/connection.cpp b/src/base/http/connection.cpp index 4a96295e4..32a7ce840 100644 --- a/src/base/http/connection.cpp +++ b/src/base/http/connection.cpp @@ -44,6 +44,7 @@ Connection::Connection(QTcpSocket *socket, IRequestHandler *requestHandler, QObj , m_requestHandler(requestHandler) { m_socket->setParent(this); + connect(m_socket, &QAbstractSocket::disconnected, this, &Connection::closed); // reserve common size for requests, don't use the max allowed size which is too big for // memory constrained platforms @@ -62,11 +63,6 @@ Connection::Connection(QTcpSocket *socket, IRequestHandler *requestHandler, QObj }); } -Connection::~Connection() -{ - m_socket->close(); -} - void Connection::read() { // reuse existing buffer and avoid unnecessary memory allocation/relocation @@ -182,11 +178,6 @@ bool Connection::hasExpired(const qint64 timeout) const && m_idleTimer.hasExpired(timeout); } -bool Connection::isClosed() const -{ - return (m_socket->state() == QAbstractSocket::UnconnectedState); -} - bool Connection::acceptsGzipEncoding(QString codings) { // [rfc7231] 5.3.4. Accept-Encoding diff --git a/src/base/http/connection.h b/src/base/http/connection.h index 5b8f67a78..46a263106 100644 --- a/src/base/http/connection.h +++ b/src/base/http/connection.h @@ -47,10 +47,11 @@ namespace Http public: Connection(QTcpSocket *socket, IRequestHandler *requestHandler, QObject *parent = nullptr); - ~Connection(); bool hasExpired(qint64 timeout) const; - bool isClosed() const; + + signals: + void closed(); private: static bool acceptsGzipEncoding(QString codings); diff --git a/src/base/http/server.cpp b/src/base/http/server.cpp index 1e2320e11..19233dc27 100644 --- a/src/base/http/server.cpp +++ b/src/base/http/server.cpp @@ -32,7 +32,10 @@ #include #include +#include +#include +#include #include #include #include @@ -40,7 +43,6 @@ #include #include -#include "base/algorithm.h" #include "base/global.h" #include "base/utils/net.h" #include "base/utils/sslkey.h" @@ -113,32 +115,38 @@ Server::Server(IRequestHandler *requestHandler, QObject *parent) void Server::incomingConnection(const qintptr socketDescriptor) { - if (m_connections.size() >= CONNECTIONS_LIMIT) return; - - QTcpSocket *serverSocket = nullptr; - if (m_https) - serverSocket = new QSslSocket(this); - else - serverSocket = new QTcpSocket(this); - + std::unique_ptr serverSocket = m_https ? std::make_unique(this) : std::make_unique(this); if (!serverSocket->setSocketDescriptor(socketDescriptor)) + return; + + if (m_connections.size() >= CONNECTIONS_LIMIT) { - delete serverSocket; + qWarning("Too many connections. Exceeded CONNECTIONS_LIMIT (%d). Connection closed.", CONNECTIONS_LIMIT); return; } - if (m_https) + try { - static_cast(serverSocket)->setProtocol(QSsl::SecureProtocols); - static_cast(serverSocket)->setPrivateKey(m_key); - static_cast(serverSocket)->setLocalCertificateChain(m_certificates); - static_cast(serverSocket)->setPeerVerifyMode(QSslSocket::VerifyNone); - static_cast(serverSocket)->startServerEncryption(); - } + if (m_https) + { + auto *sslSocket = static_cast(serverSocket.get()); + sslSocket->setProtocol(QSsl::SecureProtocols); + sslSocket->setPrivateKey(m_key); + sslSocket->setLocalCertificateChain(m_certificates); + sslSocket->setPeerVerifyMode(QSslSocket::VerifyNone); + sslSocket->startServerEncryption(); + } - auto *c = new Connection(serverSocket, m_requestHandler, this); - m_connections.insert(c); - connect(serverSocket, &QAbstractSocket::disconnected, this, [c, this]() { removeConnection(c); }); + auto *connection = new Connection(serverSocket.release(), m_requestHandler, this); + m_connections.insert(connection); + connect(connection, &Connection::closed, this, [this, connection] { removeConnection(connection); }); + } + catch (const std::bad_alloc &exception) + { + // drop the connection instead of throwing exception and crash + qWarning("Failed to allocate memory for HTTP connection. Connection closed."); + return; + } } void Server::removeConnection(Connection *connection) diff --git a/src/gui/transferlistfilterswidget.cpp b/src/gui/transferlistfilterswidget.cpp index feb93d1a6..348290ace 100644 --- a/src/gui/transferlistfilterswidget.cpp +++ b/src/gui/transferlistfilterswidget.cpp @@ -39,7 +39,6 @@ #include #include -#include "base/algorithm.h" #include "base/bittorrent/session.h" #include "base/bittorrent/torrent.h" #include "base/bittorrent/trackerentrystatus.h"