diff --git a/src/web/ng/Server.cpp b/src/web/ng/Server.cpp index 85d162e39..adef90a82 100644 --- a/src/web/ng/Server.cpp +++ b/src/web/ng/Server.cpp @@ -114,6 +114,7 @@ makeConnection( std::string ip, util::TagDecoratorFactory& tagDecoratorFactory, Server::OnConnectCheck onConnectCheck, + size_t maxSendingQueueSize, boost::asio::yield_context yield ) { @@ -130,7 +131,8 @@ makeConnection( std::move(ip), std::move(sslDetectionResult.buffer), *sslContext, - tagDecoratorFactory + tagDecoratorFactory, + maxSendingQueueSize ); sslConnection->setTimeout(std::chrono::seconds{10}); auto const expectedSuccess = sslConnection->sslHandshake(yield); @@ -146,7 +148,8 @@ makeConnection( std::move(sslDetectionResult.socket), std::move(ip), std::move(sslDetectionResult.buffer), - tagDecoratorFactory + tagDecoratorFactory, + maxSendingQueueSize ); } @@ -163,6 +166,7 @@ std::expected tryUpgradeConnection( impl::UpgradableConnectionPtr connection, util::TagDecoratorFactory& tagDecoratorFactory, + size_t maxSendingQueueSize, boost::asio::yield_context yield ) { @@ -174,7 +178,8 @@ tryUpgradeConnection( } if (*expectedIsUpgrade) { - auto expectedUpgradedConnection = connection->upgrade(tagDecoratorFactory, yield); + auto expectedUpgradedConnection = + connection->upgrade(tagDecoratorFactory, maxSendingQueueSize, yield); if (expectedUpgradedConnection.has_value()) return std::move(expectedUpgradedConnection).value(); @@ -196,13 +201,14 @@ Server::Server( std::optional parallelRequestLimit, util::TagDecoratorFactory tagDecoratorFactory, ProxyIpResolver proxyIpResolver, - std::optional maxSubscriptionSendQueueSize, + size_t maxSubscriptionSendQueueSize, Hooks hooks ) : ctx_{ctx} , sslContext_{std::move(sslContext)} , tagDecoratorFactory_{tagDecoratorFactory} - , connectionHandler_{processingPolicy, parallelRequestLimit, tagDecoratorFactory_, maxSubscriptionSendQueueSize, std::move(proxyIpResolver), std::move(hooks.onDisconnectHook), std::move(hooks.onIpChangeHook)} + , maxSubscriptionSendQueueSize_{maxSubscriptionSendQueueSize} + , connectionHandler_{processingPolicy, parallelRequestLimit, tagDecoratorFactory_, std::move(proxyIpResolver), std::move(hooks.onDisconnectHook), std::move(hooks.onIpChangeHook)} , endpoint_{std::move(endpoint)} , onConnectCheck_{std::move(hooks.onConnectCheck)} { @@ -296,6 +302,7 @@ Server::handleConnection(boost::asio::ip::tcp::socket socket, boost::asio::yield std::move(ip).value(), tagDecoratorFactory_, onConnectCheck_, + maxSubscriptionSendQueueSize_, yield ); if (not connectionExpected.has_value()) { @@ -318,8 +325,12 @@ Server::handleConnection(boost::asio::ip::tcp::socket socket, boost::asio::yield return; } - auto connection = - tryUpgradeConnection(std::move(connectionExpected).value(), tagDecoratorFactory_, yield); + auto connection = tryUpgradeConnection( + std::move(connectionExpected).value(), + tagDecoratorFactory_, + maxSubscriptionSendQueueSize_, + yield + ); if (not connection.has_value()) { LOG(log_.info()) << connection.error(); return; diff --git a/src/web/ng/Server.hpp b/src/web/ng/Server.hpp index 358cb617e..696d30ba2 100644 --- a/src/web/ng/Server.hpp +++ b/src/web/ng/Server.hpp @@ -59,6 +59,7 @@ private: std::optional sslContext_; util::TagDecoratorFactory tagDecoratorFactory_; + size_t maxSubscriptionSendQueueSize_; impl::ConnectionHandler connectionHandler_; boost::asio::ip::tcp::endpoint endpoint_; @@ -90,7 +91,7 @@ public: std::optional parallelRequestLimit, util::TagDecoratorFactory tagDecoratorFactory, ProxyIpResolver proxyIpResolver, - std::optional maxSubscriptionSendQueueSize, + size_t maxSubscriptionSendQueueSize, Hooks hooks ); diff --git a/src/web/ng/SubscriptionContext.cpp b/src/web/ng/SubscriptionContext.cpp index f59331881..c77a6a98a 100644 --- a/src/web/ng/SubscriptionContext.cpp +++ b/src/web/ng/SubscriptionContext.cpp @@ -6,10 +6,8 @@ #include -#include #include #include -#include #include #include @@ -18,13 +16,11 @@ namespace web::ng { SubscriptionContext::SubscriptionContext( util::TagDecoratorFactory const& factory, impl::WsConnectionBase& connection, - std::optional maxSendQueueSize, boost::asio::yield_context yield, ErrorHandler errorHandler ) : web::SubscriptionContextInterface(factory) , connection_(connection) - , maxSendQueueSize_(maxSendQueueSize) , tasksGroup_(yield) , yield_(yield) , errorHandler_(std::move(errorHandler)) @@ -42,14 +38,6 @@ SubscriptionContext::send(std::shared_ptr message) if (disconnected_ or gotError_) return; - if (maxSendQueueSize_.has_value() and tasksGroup_.size() >= *maxSendQueueSize_) { - tasksGroup_.spawn(yield_, [this](boost::asio::yield_context innerYield) { - connection_.get().close(innerYield); - }); - gotError_ = true; - return; - } - tasksGroup_.spawn( yield_, [this, message = std::move(message)](boost::asio::yield_context innerYield) mutable { diff --git a/src/web/ng/SubscriptionContext.hpp b/src/web/ng/SubscriptionContext.hpp index 54be02377..ba31d4da3 100644 --- a/src/web/ng/SubscriptionContext.hpp +++ b/src/web/ng/SubscriptionContext.hpp @@ -12,11 +12,9 @@ #include #include -#include #include #include #include -#include #include namespace web::ng { @@ -36,7 +34,6 @@ public: private: std::reference_wrapper connection_; - std::optional maxSendQueueSize_; util::CoroutineGroup tasksGroup_; boost::asio::yield_context yield_; ErrorHandler errorHandler_; @@ -59,15 +56,12 @@ public: * * @param factory The tag decorator factory to use to init taggable. * @param connection The connection for which the context is created. - * @param maxSendQueueSize The maximum size of the send queue. If the queue is full, the - * connection will be closed. * @param yield The yield context to spawn sending coroutines. * @param errorHandler The error handler. */ SubscriptionContext( util::TagDecoratorFactory const& factory, impl::WsConnectionBase& connection, - std::optional maxSendQueueSize, boost::asio::yield_context yield, ErrorHandler errorHandler ); diff --git a/src/web/ng/impl/ConnectionHandler.cpp b/src/web/ng/impl/ConnectionHandler.cpp index ef3552b39..d3bb9e328 100644 --- a/src/web/ng/impl/ConnectionHandler.cpp +++ b/src/web/ng/impl/ConnectionHandler.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -75,7 +76,6 @@ ConnectionHandler::ConnectionHandler( ProcessingPolicy processingPolicy, std::optional maxParallelRequests, util::TagDecoratorFactory& tagFactory, - std::optional maxSubscriptionSendQueueSize, ProxyIpResolver proxyIpResolver, OnDisconnectHook onDisconnectHook, OnIpChangeHook onIpChangeHook @@ -83,7 +83,6 @@ ConnectionHandler::ConnectionHandler( : processingPolicy_{processingPolicy} , maxParallelRequests_{maxParallelRequests} , tagFactory_{tagFactory} - , maxSubscriptionSendQueueSize_{maxSubscriptionSendQueueSize} , proxyIpResolver_(std::move(proxyIpResolver)) , onDisconnectHook_{std::move(onDisconnectHook)} , onIpChangeHook_(std::move(onIpChangeHook)) @@ -136,11 +135,7 @@ ConnectionHandler::processConnection(ConnectionPtr connectionPtr, boost::asio::y auto* ptr = dynamic_cast(connectionPtr.get()); ASSERT(ptr != nullptr, "Casted not websocket connection"); subscriptionContext = std::make_shared( - tagFactory_, - *ptr, - maxSubscriptionSendQueueSize_, - yield, - [this](Error const& e, Connection const& c) { return handleError(e, c); } + tagFactory_, *ptr, yield, std::bind_front(&ConnectionHandler::handleError, this) ); LOG(log_.trace()) << connectionRef.tag() << "Created SubscriptionContext for the connection"; diff --git a/src/web/ng/impl/ConnectionHandler.hpp b/src/web/ng/impl/ConnectionHandler.hpp index 31068ac7f..d2911e778 100644 --- a/src/web/ng/impl/ConnectionHandler.hpp +++ b/src/web/ng/impl/ConnectionHandler.hpp @@ -46,7 +46,6 @@ private: std::optional maxParallelRequests_; std::reference_wrapper tagFactory_; - std::optional maxSubscriptionSendQueueSize_; ProxyIpResolver proxyIpResolver_; @@ -73,7 +72,6 @@ public: ProcessingPolicy processingPolicy, std::optional maxParallelRequests, util::TagDecoratorFactory& tagFactory, - std::optional maxSubscriptionSendQueueSize, ProxyIpResolver proxyIpResolver, OnDisconnectHook onDisconnectHook, OnIpChangeHook onIpChangeHook diff --git a/src/web/ng/impl/HttpConnection.hpp b/src/web/ng/impl/HttpConnection.hpp index ca1bdb7bb..213e28c6b 100644 --- a/src/web/ng/impl/HttpConnection.hpp +++ b/src/web/ng/impl/HttpConnection.hpp @@ -26,6 +26,7 @@ #include #include +#include #include #include #include @@ -43,6 +44,7 @@ public: virtual std::expected upgrade( util::TagDecoratorFactory const& tagDecoratorFactory, + size_t maxSendingQueueSize, boost::asio::yield_context yield ) = 0; @@ -71,15 +73,19 @@ public: boost::asio::ip::tcp::socket socket, std::string ip, boost::beast::flat_buffer buffer, - util::TagDecoratorFactory const& tagDecoratorFactory + util::TagDecoratorFactory const& tagDecoratorFactory, + size_t maxSendingQueueSize ) requires IsTcpStream : UpgradableConnection(std::move(ip), std::move(buffer), tagDecoratorFactory) , stream_{std::move(socket)} - , sendingQueue_([this](MessageType const& message, auto&& yield) { - boost::beast::get_lowest_layer(stream_).expires_after(timeout_); - boost::beast::http::async_write(stream_, message, yield); - }) + , sendingQueue_( + [this](MessageType const& message, auto&& yield) { + boost::beast::get_lowest_layer(stream_).expires_after(timeout_); + boost::beast::http::async_write(stream_, message, yield); + }, + maxSendingQueueSize + ) { } @@ -88,15 +94,19 @@ public: std::string ip, boost::beast::flat_buffer buffer, boost::asio::ssl::context& sslCtx, - util::TagDecoratorFactory const& tagDecoratorFactory + util::TagDecoratorFactory const& tagDecoratorFactory, + size_t maxSendingQueueSize ) requires IsSslTcpStream : UpgradableConnection(std::move(ip), std::move(buffer), tagDecoratorFactory) , stream_{std::move(socket), sslCtx} - , sendingQueue_([this](MessageType const& message, auto&& yield) { - boost::beast::get_lowest_layer(stream_).expires_after(timeout_); - boost::beast::http::async_write(stream_, message, yield); - }) + , sendingQueue_( + [this](MessageType const& message, auto&& yield) { + boost::beast::get_lowest_layer(stream_).expires_after(timeout_); + boost::beast::http::async_write(stream_, message, yield); + }, + maxSendingQueueSize + ) { } @@ -202,6 +212,7 @@ public: std::expected upgrade( util::TagDecoratorFactory const& tagDecoratorFactory, + size_t maxSendingQueueSize, boost::asio::yield_context yield ) override { @@ -213,6 +224,7 @@ public: std::move(buffer_), std::move(*request_), // NOLINT(bugprone-unchecked-optional-access) tagDecoratorFactory, + maxSendingQueueSize, yield ); } diff --git a/src/web/ng/impl/SendingQueue.hpp b/src/web/ng/impl/SendingQueue.hpp index 6c83593bb..ae43e9528 100644 --- a/src/web/ng/impl/SendingQueue.hpp +++ b/src/web/ng/impl/SendingQueue.hpp @@ -3,12 +3,15 @@ #include "web/ng/Error.hpp" #include +#include #include #include +#include +#include #include -#include #include +#include namespace web::ng::impl { @@ -23,9 +26,10 @@ private: Sender sender_; Error error_; bool isSending_{false}; + size_t maxSize_; public: - SendingQueue(Sender sender) : sender_{std::move(sender)} + SendingQueue(Sender sender, size_t maxSize) : sender_{std::move(sender)}, maxSize_{maxSize} { } @@ -35,6 +39,11 @@ public: if (error_) return std::unexpected{error_}; + if (queue_.size() >= maxSize_) { + error_ = boost::asio::error::timed_out; + return std::unexpected{error_}; + } + queue_.push(std::move(message)); if (isSending_) return {}; @@ -43,7 +52,11 @@ public: while (not queue_.empty() and not error_) { auto const responseToSend = std::move(queue_.front()); queue_.pop(); - sender_(responseToSend, yield[error_]); + + Error writeError; + sender_(responseToSend, yield[writeError]); + if (writeError) + error_ = writeError; } isSending_ = false; if (error_) diff --git a/src/web/ng/impl/WsConnection.hpp b/src/web/ng/impl/WsConnection.hpp index b009ef74e..aea4b3ef1 100644 --- a/src/web/ng/impl/WsConnection.hpp +++ b/src/web/ng/impl/WsConnection.hpp @@ -28,8 +28,8 @@ #include #include +#include #include -#include #include #include #include @@ -60,23 +60,29 @@ public: std::string ip, boost::beast::flat_buffer buffer, boost::beast::http::request initialRequest, - util::TagDecoratorFactory const& tagDecoratorFactory + util::TagDecoratorFactory const& tagDecoratorFactory, + size_t maxSendingQueueSize ) : WsConnectionBase(std::move(ip), std::move(buffer), tagDecoratorFactory) , stream_(std::move(stream)) , initialRequest_(std::move(initialRequest)) - , sendingQueue_{[this](MessageType const& message, auto&& yield) { - boost::asio::const_buffer const buffer = std::visit( - util::OverloadSet{ - [](Response const& r) -> boost::asio::const_buffer { return r.asWsResponse(); }, - [](std::shared_ptr const& m) -> boost::asio::const_buffer { - return boost::asio::buffer(*m); - } - }, - message - ); - stream_.async_write(buffer, yield); - }} + , sendingQueue_{ + [this](MessageType const& message, auto&& yield) { + boost::asio::const_buffer const buffer = std::visit( + util::OverloadSet{ + [](Response const& r) -> boost::asio::const_buffer { + return r.asWsResponse(); + }, + [](std::shared_ptr const& m) -> boost::asio::const_buffer { + return boost::asio::buffer(*m); + } + }, + message + ); + stream_.async_write(buffer, yield); + }, + maxSendingQueueSize + } { setupWsStream(); } @@ -188,6 +194,7 @@ makeWsConnection( boost::beast::flat_buffer buffer, boost::beast::http::request request, util::TagDecoratorFactory const& tagDecoratorFactory, + size_t maxSendingQueueSize, boost::asio::yield_context yield ) { @@ -196,7 +203,8 @@ makeWsConnection( std::move(ip), std::move(buffer), std::move(request), - tagDecoratorFactory + tagDecoratorFactory, + maxSendingQueueSize ); auto const expectedSuccess = connection->performHandshake(yield); if (not expectedSuccess.has_value()) diff --git a/tests/common/web/ng/impl/MockHttpConnection.hpp b/tests/common/web/ng/impl/MockHttpConnection.hpp index 00858f6a2..6afc442f1 100644 --- a/tests/common/web/ng/impl/MockHttpConnection.hpp +++ b/tests/common/web/ng/impl/MockHttpConnection.hpp @@ -14,6 +14,7 @@ #include #include +#include #include #include @@ -52,7 +53,9 @@ struct MockHttpConnectionImpl : web::ng::impl::UpgradableConnection { MOCK_METHOD( UpgradeReturnType, upgrade, - (util::TagDecoratorFactory const& tagDecoratorFactory, boost::asio::yield_context yield), + (util::TagDecoratorFactory const& tagDecoratorFactory, + size_t maxSendingQueueSize, + boost::asio::yield_context yield), (override) ); }; diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index e2dc42df0..2ee4baeb5 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -213,6 +213,7 @@ target_sources( web/ng/impl/ConnectionHandlerTests.cpp web/ng/impl/ErrorHandlingTests.cpp web/ng/impl/HttpConnectionTests.cpp + web/ng/impl/SendingQueueTests.cpp web/ng/impl/ServerSslContextTests.cpp web/ng/impl/WsConnectionTests.cpp web/ProxyIpResolverTests.cpp diff --git a/tests/unit/web/ng/ServerTests.cpp b/tests/unit/web/ng/ServerTests.cpp index 21faffebd..2ca8558fa 100644 --- a/tests/unit/web/ng/ServerTests.cpp +++ b/tests/unit/web/ng/ServerTests.cpp @@ -35,12 +35,17 @@ #include #include +#include #include #include #include #include #include +namespace { +constexpr size_t kMaxSendingQueueSize = 1500; +} // namespace + using namespace web::ng; using namespace util::config; @@ -218,7 +223,7 @@ TEST_F(ServerTest, BadEndpoint) std::nullopt, tagDecoratorFactory, web::ProxyIpResolver{{}, {}}, - std::nullopt, + kMaxSendingQueueSize, Server::Hooks{ .onConnectCheck = emptyOnConnectCheck_, .onIpChangeHook = [](auto&&, auto&&) {}, @@ -291,7 +296,7 @@ TEST_F(ServerHttpTest, OnConnectCheck) std::nullopt, tagDecoratorFactory, web::ProxyIpResolver{{}, {}}, - std::nullopt, + kMaxSendingQueueSize, Server::Hooks{ .onConnectCheck = onConnectCheck.AsStdFunction(), .onIpChangeHook = [](auto&&, auto&&) {}, @@ -361,7 +366,7 @@ TEST_F(ServerHttpTest, OnConnectCheckFailed) std::nullopt, tagDecoratorFactory, web::ProxyIpResolver{{}, {}}, - std::nullopt, + kMaxSendingQueueSize, Server::Hooks{ .onConnectCheck = onConnectCheck.AsStdFunction(), .onIpChangeHook = [](auto&&, auto&&) {}, @@ -431,7 +436,7 @@ TEST_F(ServerHttpTest, OnDisconnectHook) std::nullopt, tagDecoratorFactory, web::ProxyIpResolver{{}, {}}, - std::nullopt, + kMaxSendingQueueSize, Server::Hooks{ .onConnectCheck = emptyOnConnectCheck_, .onIpChangeHook = [](auto&&, auto&&) {}, diff --git a/tests/unit/web/ng/SubscriptionContextTests.cpp b/tests/unit/web/ng/SubscriptionContextTests.cpp index c553ff9f6..1d1d7bf34 100644 --- a/tests/unit/web/ng/SubscriptionContextTests.cpp +++ b/tests/unit/web/ng/SubscriptionContextTests.cpp @@ -10,16 +10,14 @@ #include "web/ng/SubscriptionContext.hpp" #include "web/ng/impl/MockWsConnection.hpp" -#include +#include #include #include #include #include #include -#include #include -#include #include using namespace web::ng; @@ -27,14 +25,9 @@ using namespace util::config; struct NgSubscriptionContextTests : SyncAsioContextTest { SubscriptionContext - makeSubscriptionContext( - boost::asio::yield_context yield, - std::optional maxSendQueueSize = std::nullopt - ) + makeSubscriptionContext(boost::asio::yield_context yield) { - return SubscriptionContext{ - tagFactory_, connection_, maxSendQueueSize, yield, errorHandler_.AsStdFunction() - }; + return SubscriptionContext{tagFactory_, connection_, yield, errorHandler_.AsStdFunction()}; } protected: @@ -124,26 +117,15 @@ TEST_F(NgSubscriptionContextTests, SendFailed) TEST_F(NgSubscriptionContextTests, SendTooManySubscriptions) { runSpawn([this](boost::asio::yield_context yield) { - auto subscriptionContext = makeSubscriptionContext(yield, 1); + auto subscriptionContext = makeSubscriptionContext(yield); auto const message = std::make_shared("message1"); - EXPECT_CALL(connection_, sendShared) - .WillOnce( - [&message]( - std::shared_ptr sendingMessage, - boost::asio::yield_context innerYield - ) -> std::expected { - boost::asio::post( - innerYield - ); // simulate send is slow by switching to another coroutine - EXPECT_EQ(sendingMessage, message); - return {}; - } - ); + EXPECT_CALL(connection_, sendShared).WillOnce([](auto&&, auto&&) { + return std::unexpected{boost::system::error_code{boost::asio::error::timed_out}}; + }); + EXPECT_CALL(errorHandler_, Call).WillOnce(testing::Return(true)); EXPECT_CALL(connection_, close); - subscriptionContext.send(message); - subscriptionContext.send(message); subscriptionContext.send(message); subscriptionContext.disconnect(yield); }); diff --git a/tests/unit/web/ng/impl/ConnectionHandlerTests.cpp b/tests/unit/web/ng/impl/ConnectionHandlerTests.cpp index 28d4cb155..e2b1edfc5 100644 --- a/tests/unit/web/ng/impl/ConnectionHandlerTests.cpp +++ b/tests/unit/web/ng/impl/ConnectionHandlerTests.cpp @@ -56,7 +56,6 @@ struct ConnectionHandlerTest : prometheus::WithPrometheus, SyncAsioContextTest { policy, maxParallelConnections, tagFactory, - std::nullopt, proxyIpResolver, onDisconnectMock.AsStdFunction(), onIpChangeMock.AsStdFunction() diff --git a/tests/unit/web/ng/impl/HttpConnectionTests.cpp b/tests/unit/web/ng/impl/HttpConnectionTests.cpp index c573f9473..805aef306 100644 --- a/tests/unit/web/ng/impl/HttpConnectionTests.cpp +++ b/tests/unit/web/ng/impl/HttpConnectionTests.cpp @@ -29,6 +29,10 @@ #include #include +namespace { +constexpr size_t kMaxSendingQueueSize = 1500; +} // namespace + using namespace web::ng::impl; using namespace web::ng; using namespace util::config; @@ -45,7 +49,8 @@ struct HttpConnectionTests : SyncAsioContextTest { std::move(expectedSocket).value(), std::move(ip), boost::beast::flat_buffer{}, - tagDecoratorFactory_ + tagDecoratorFactory_, + kMaxSendingQueueSize ); connection->setTimeout(std::chrono::milliseconds{100}); return connection; @@ -388,7 +393,8 @@ TEST_F(HttpConnectionTests, Upgrade) [&]() { ASSERT_TRUE(expectedResult.has_value()) << expectedResult.error().message(); }(); [&]() { ASSERT_TRUE(expectedResult.value()); }(); - auto expectedWsConnection = connection->upgrade(tagDecoratorFactory_, yield); + auto expectedWsConnection = + connection->upgrade(tagDecoratorFactory_, kMaxSendingQueueSize, yield); [&]() { ASSERT_TRUE(expectedWsConnection.has_value()) << expectedWsConnection.error().message(); }(); diff --git a/tests/unit/web/ng/impl/SendingQueueTests.cpp b/tests/unit/web/ng/impl/SendingQueueTests.cpp new file mode 100644 index 000000000..c9b1eddf4 --- /dev/null +++ b/tests/unit/web/ng/impl/SendingQueueTests.cpp @@ -0,0 +1,152 @@ +#include "util/AsioContextTestFixture.hpp" +#include "util/CoroutineGroup.hpp" +#include "web/ng/impl/SendingQueue.hpp" + +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +namespace { +constexpr size_t kLargeLimit = 10000; +} // namespace + +using namespace web::ng::impl; + +struct SendingQueueTests : SyncAsioContextTest {}; + +TEST_F(SendingQueueTests, SendsInOrder) +{ + std::vector sent; + SendingQueue queue{ + [&sent](std::string const& message, auto&&) { sent.push_back(message); }, kLargeLimit + }; + + runSpawn([&queue](boost::asio::yield_context yield) { + EXPECT_TRUE(queue.send("one", yield).has_value()); + EXPECT_TRUE(queue.send("two", yield).has_value()); + }); + + EXPECT_EQ(sent, (std::vector{"one", "two"})); +} + +// While one coroutine drains the queue, every other sender only pushes and returns - so the number +// of live coroutines says nothing about how many messages are pending. +TEST_F(SendingQueueTests, NonDrainingSendersOnlyPushAndReturn) +{ + constexpr size_t kSendersWhileBlocked = 100; + size_t sentCount = 0; + + SendingQueue queue{ + [&sentCount](std::string const&, auto&& yield) { + boost::asio::post(yield); // the peer is slow: suspend inside the drain loop + ++sentCount; + }, + kLargeLimit + }; + + runSpawn([&queue](boost::asio::yield_context yield) { + util::CoroutineGroup group{yield}; + for (size_t i = 0; i <= kSendersWhileBlocked; ++i) { + group.spawn(yield, [&queue](boost::asio::yield_context innerYield) { + EXPECT_TRUE(queue.send("message", innerYield).has_value()); + }); + } + group.asyncWait(yield); + }); + + EXPECT_EQ(sentCount, kSendersWhileBlocked + 1); +} + +// The regression test: with a limit set, the sender that would push past it is rejected with +// timed_out instead of growing the queue. Before the fix SendingQueue had no limit at all and every +// one of these senders succeeded. +TEST_F(SendingQueueTests, RejectsWhenFull) +{ + constexpr size_t kMaxSize = 4; + size_t sentCount = 0; + size_t rejectedCount = 0; + + SendingQueue queue{ + [&sentCount](std::string const&, auto&& yield) { + boost::asio::post(yield); // the peer is slow: suspend inside the drain loop + ++sentCount; + }, + kMaxSize + }; + + runSpawn([&queue, &rejectedCount](boost::asio::yield_context yield) { + util::CoroutineGroup group{yield}; + for (size_t i = 0; i < kMaxSize * 4; ++i) { + group.spawn(yield, [&queue, &rejectedCount](boost::asio::yield_context innerYield) { + auto const result = queue.send("message", innerYield); + if (not result.has_value()) { + EXPECT_EQ(result.error(), boost::asio::error::timed_out); + ++rejectedCount; + } + }); + } + group.asyncWait(yield); + }); + + EXPECT_GT(rejectedCount, 0u) << "the limit must reject senders once the queue is full"; + EXPECT_LE(sentCount, kMaxSize + 1) << "no more than the limit may ever be pending"; +} + +// Once the limit has been hit the connection is doomed, so every later send fails too rather than +// silently resuming. +TEST_F(SendingQueueTests, StaysFailedAfterRejection) +{ + SendingQueue queue{ + [](std::string const&, auto&& yield) { boost::asio::post(yield); }, 1 + }; + + runSpawn([&queue](boost::asio::yield_context yield) { + util::CoroutineGroup group{yield}; + for (size_t i = 0; i < 4; ++i) { + group.spawn(yield, [&queue](boost::asio::yield_context innerYield) { + queue.send("message", innerYield); + }); + } + group.asyncWait(yield); + + auto const result = queue.send("after", yield); + ASSERT_FALSE(result.has_value()); + EXPECT_EQ(result.error(), boost::asio::error::timed_out); + }); +} + +// Regression: the overflow state must survive the completion of a write that was already in +// flight. Real senders complete with void(error_code), so asio writes success into whatever error +// slot the drain loop bound for that write - which must not be the slot holding the overflow. +TEST_F(SendingQueueTests, OverflowSurvivesSuccessfulInFlightWrite) +{ + boost::asio::steady_timer timer{ctx_}; + SendingQueue queue{ + [&timer](std::string const&, auto&& yield) { + timer.expires_after(std::chrono::milliseconds{1}); + timer.async_wait(yield); // void(error_code), and it succeeds + }, + 1 + }; + + runSpawn([&queue](boost::asio::yield_context yield) { + util::CoroutineGroup group{yield}; + for (size_t i = 0; i < 4; ++i) { + group.spawn(yield, [&queue](boost::asio::yield_context innerYield) { + queue.send("message", innerYield); + }); + } + group.asyncWait(yield); + + auto const result = queue.send("after", yield); + ASSERT_FALSE(result.has_value()) << "queue must stay failed once it has overflowed"; + EXPECT_EQ(result.error(), boost::asio::error::timed_out); + }); +} diff --git a/tests/unit/web/ng/impl/WsConnectionTests.cpp b/tests/unit/web/ng/impl/WsConnectionTests.cpp index 64c8b9f55..7164d9a08 100644 --- a/tests/unit/web/ng/impl/WsConnectionTests.cpp +++ b/tests/unit/web/ng/impl/WsConnectionTests.cpp @@ -33,6 +33,10 @@ #include #include +namespace { +constexpr size_t kMaxSendingQueueSize = 1500; +} // namespace + using namespace web::ng::impl; using namespace web::ng; using namespace util; @@ -50,6 +54,7 @@ struct WebWsConnectionTests : SyncAsioContextTest { std::move(ip), boost::beast::flat_buffer{}, tagDecoratorFactory_, + kMaxSendingQueueSize, }; auto expectedTrue = httpConnection.isUpgradeRequested(yield); @@ -58,7 +63,8 @@ struct WebWsConnectionTests : SyncAsioContextTest { ASSERT_TRUE(expectedTrue.value()) << "Expected upgrade request"; }(); - auto expectedWsConnection = httpConnection.upgrade(tagDecoratorFactory_, yield); + auto expectedWsConnection = + httpConnection.upgrade(tagDecoratorFactory_, kMaxSendingQueueSize, yield); [&]() { ASSERT_TRUE(expectedWsConnection.has_value()) << expectedWsConnection.error().message(); }();