Skip to content

Commit

Permalink
Fixes issue 181.
Browse files Browse the repository at this point in the history
  • Loading branch information
mzimbres committed Feb 6, 2024
1 parent 0445e74 commit a6b8017
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 1 deletion.
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,11 @@ https://lists.boost.org/Archives/boost/2023/01/253944.php.
apps need only one connection for their entire application, which
makes the overhead of one ssl-context per connection negligible.

* ([Issue 181](https://github.com/boostorg/redis/issues/181)).
See a detailed description of this bug in
[this](https://github.com/boostorg/redis/issues/181#issuecomment-1913346983)
comment.

### Boost 1.84 (First release in Boost)

* Deprecates the `async_receive` overload that takes a response. Users
Expand Down
15 changes: 14 additions & 1 deletion include/boost/redis/detail/connection_base.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,9 @@ class connection_base {
usage get_usage() const noexcept
{ return usage_; }

auto run_is_canceled() const noexcept
{ return cancel_run_called_; }

private:
using receive_channel_type = asio::experimental::channel<executor_type, void(system::error_code, std::size_t)>;
using runner_type = runner<executor_type>;
Expand Down Expand Up @@ -539,6 +542,7 @@ class connection_base {
});

reqs_.erase(point, std::end(reqs_));

std::for_each(std::begin(reqs_), std::end(reqs_), [](auto const& ptr) {
return ptr->mark_waiting();
});
Expand Down Expand Up @@ -575,6 +579,12 @@ class connection_base {
} break;
case operation::run:
{
// Protects the code below from being called more than
// once, see https://github.com/boostorg/redis/issues/181
if (std::exchange(cancel_run_called_, true)) {
return;
}

close();
writer_timer_.cancel();
receive_channel_.cancel();
Expand All @@ -601,8 +611,9 @@ class connection_base {
// partition of unwritten requests instead of them all.
std::for_each(std::begin(reqs_), std::end(reqs_), [](auto const& ptr) {
BOOST_ASSERT_MSG(ptr != nullptr, "Expects non-null pointer.");
if (ptr->is_staged())
if (ptr->is_staged()) {
ptr->mark_written();
}
});
}

Expand Down Expand Up @@ -921,6 +932,7 @@ class connection_base {
read_buffer_.clear();
parser_.reset();
on_push_ = false;
cancel_run_called_ = false;
}

asio::ssl::context ctx_;
Expand All @@ -942,6 +954,7 @@ class connection_base {
reqs_type reqs_;
resp3::parser parser_{};
bool on_push_ = false;
bool cancel_run_called_ = false;

usage usage_;
};
Expand Down
1 change: 1 addition & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ make_test(test_conn_exec_cancel2 20)
make_test(test_conn_echo_stress 20)
make_test(test_conn_run_cancel 20)
make_test(test_issue_50 20)
make_test(test_issue_181 17)

# Coverage
set(
Expand Down
64 changes: 64 additions & 0 deletions test/test_issue_181.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/* Copyright (c) 2018-2024 Marcelo Zimbres Silva ([email protected])
*
* Distributed under the Boost Software License, Version 1.0. (See
* accompanying file LICENSE.txt)
*/

#include <boost/redis/connection.hpp>
#include <boost/redis/logger.hpp>
#include <boost/asio/awaitable.hpp>
#include <boost/asio/use_awaitable.hpp>
#define BOOST_TEST_MODULE conn-quit
#include <boost/test/included/unit_test.hpp>
#include <chrono>
#include <iostream>
#include <memory>
#include <optional>
#include <string>
#include "common.hpp"

namespace net = boost::asio;
using boost::redis::request;
using boost::redis::request;
using boost::redis::response;
using boost::redis::ignore;
using boost::redis::logger;
using boost::redis::config;
using boost::redis::operation;
using boost::redis::connection;
using boost::system::error_code;
using namespace std::chrono_literals;

BOOST_AUTO_TEST_CASE(issue_181)
{
using connection_base = boost::redis::detail::connection_base<net::any_io_executor>;

auto const level = boost::redis::logger::level::debug;
net::io_context ioc;
auto ctx = net::ssl::context{net::ssl::context::tlsv12_client};
connection_base conn{ioc.get_executor(), std::move(ctx), 1000000};
net::steady_timer timer{ioc};
timer.expires_after(std::chrono::seconds{1});

auto run_cont = [&](auto ec){
std::cout << "async_run1: " << ec.message() << std::endl;
};

auto cfg = make_test_config();
cfg.health_check_interval = std::chrono::seconds{0};
cfg.reconnect_wait_interval = std::chrono::seconds{0};
conn.async_run(cfg, boost::redis::logger{level}, run_cont);
BOOST_TEST(!conn.run_is_canceled());

// Uses a timer to wait some time until run has been called.
auto timer_cont = [&](auto ec){
std::cout << "timer_cont: " << ec.message() << std::endl;
BOOST_TEST(!conn.run_is_canceled());
conn.cancel(operation::run);
BOOST_TEST(conn.run_is_canceled());
};

timer.async_wait(timer_cont);

ioc.run();
}

0 comments on commit a6b8017

Please sign in to comment.