-
Notifications
You must be signed in to change notification settings - Fork 40
async_exec now uses a sans-io strategy #250
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
anarthal
wants to merge
41
commits into
boostorg:develop
Choose a base branch
from
anarthal:feature/sansio-exec
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
41 commits
Select commit
Hold shift + click to select a range
4557269
Initial implementation
anarthal 5e227f0
Bugfixing
anarthal 0a7a0e1
adjust copyright of new files
anarthal 84bde33
Made multiplexer use default-constructed error codes for elements
anarthal 726fd92
Fix test warnings regarding signed to unsigned comparisons
anarthal cf473b7
Remove std::move inhibiting optimizations in test
anarthal 6333363
test_exec_fsm (1)
anarthal 6b8e773
memory de-allocation check
anarthal 3a7142b
write in progress test
anarthal 0e9020c
refactor
anarthal 5fe3219
cancel if not connected
anarthal f518c7b
Remove leftover runner.hpp
anarthal 936c517
writer notification is now unconditional
anarthal b4b5d32
test_not_connected
anarthal 79b4c62
better handling of cancellations
anarthal 21fd0c6
cancel waiting
anarthal a048647
cmake
anarthal 98c2450
Merge branch 'develop' into feature/sansio-exec
anarthal 8006ffb
Merge branch 'develop' into feature/sansio-exec
anarthal f494c1a
waiting, all cancel types
anarthal 889c7e7
cancel terminal not waiting
anarthal 053e464
Finish cancellation tests
anarthal cf06d26
refactor
anarthal 7977f97
Add test for cancelling pending requests
anarthal fe0e662
extend test to all cancel types
anarthal 5291e38
Enable all cancellation types for async_exec
anarthal f5c1a51
doc paragraph on cancellation
anarthal 77e8cea
use async_immediate
anarthal 5663515
classify tests in cmake
anarthal 2723640
unit tests to Jamfile
anarthal f5b3fae
Merge branch 'develop' into feature/sansio-exec
anarthal d4eda3a
commit_read => consume_next
anarthal 7820f28
Remove test_conversions from Jamfile
anarthal 9a6acec
Merge branch 'develop' into feature/sansio-exec
anarthal 8c35f41
apply PR comments
anarthal 772c7cc
explicit return type
anarthal 145dd88
(failing) parse error test
anarthal 74768f4
remove request from the multiplexer on error
anarthal 8b3fc00
Restore runner
anarthal 4af8aa9
Remove C++ std from cmake test
anarthal 102e8b0
new setup_cancellation action
anarthal File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
// | ||
// Copyright (c) 2025 Marcelo Zimbres Silva ([email protected]), | ||
// Ruben Perez Hidalgo (rubenperez038 at gmail dot com) | ||
// | ||
// Distributed under the Boost Software License, Version 1.0. (See accompanying | ||
// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) | ||
// | ||
|
||
#ifndef BOOST_REDIS_DETAIL_COROUTINE_HPP | ||
#define BOOST_REDIS_DETAIL_COROUTINE_HPP | ||
|
||
// asio::coroutine uses __COUNTER__ internally, which can trigger | ||
// ODR violations if we use them in header-only code. These manifest as | ||
// extremely hard-to-debug bugs only present in release builds. | ||
// Use this instead when doing coroutines in non-template code. | ||
// Adapted from Boost.MySQL. | ||
|
||
// Coroutine state is represented as an integer (resume_point_var). | ||
// Every yield gets assigned a unique value (resume_point_id). | ||
// Yielding sets the next resume point, returns, and sets a case label for re-entering. | ||
// Coroutines need to switch on resume_point_var to re-enter. | ||
|
||
// Enclosing this in a scope allows placing the macro inside a brace-less for/while loop | ||
// The empty scope after the case label is required because labels can't be at the end of a compound statement | ||
#define BOOST_REDIS_YIELD(resume_point_var, resume_point_id, ...) \ | ||
{ \ | ||
resume_point_var = resume_point_id; \ | ||
return __VA_ARGS__; \ | ||
case resume_point_id: \ | ||
{ \ | ||
} \ | ||
} | ||
|
||
#define BOOST_REDIS_CORO_INITIAL case 0: | ||
|
||
#endif |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,149 @@ | ||
// | ||
// Copyright (c) 2025 Marcelo Zimbres Silva ([email protected]), | ||
// Ruben Perez Hidalgo (rubenperez038 at gmail dot com) | ||
// | ||
// Distributed under the Boost Software License, Version 1.0. (See accompanying | ||
// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) | ||
// | ||
|
||
#ifndef BOOST_REDIS_EXEC_FSM_HPP | ||
#define BOOST_REDIS_EXEC_FSM_HPP | ||
|
||
#include <boost/redis/detail/coroutine.hpp> | ||
#include <boost/redis/detail/multiplexer.hpp> | ||
#include <boost/redis/request.hpp> | ||
|
||
#include <boost/asio/cancellation_type.hpp> | ||
#include <boost/asio/error.hpp> | ||
#include <boost/assert.hpp> | ||
#include <boost/system/error_code.hpp> | ||
|
||
#include <cstddef> | ||
#include <memory> | ||
|
||
// Sans-io algorithm for async_exec, as a finite state machine | ||
|
||
namespace boost::redis::detail { | ||
|
||
// What should we do next? | ||
enum class exec_action_type | ||
{ | ||
setup_cancellation, // Set up the cancellation types supported by the composed operation | ||
immediate, // Invoke asio::async_immediate to avoid re-entrancy problems | ||
done, // Call the final handler | ||
write, // Notify the writer task | ||
wait_for_response, // Wait to be notified | ||
cancel_run, // Cancel the connection's run operation | ||
}; | ||
|
||
class exec_action { | ||
exec_action_type type_; | ||
system::error_code ec_; | ||
std::size_t bytes_read_; | ||
|
||
public: | ||
exec_action(exec_action_type type) noexcept | ||
: type_{type} | ||
{ } | ||
|
||
exec_action(system::error_code ec, std::size_t bytes_read = 0u) noexcept | ||
: type_{exec_action_type::done} | ||
, ec_{ec} | ||
, bytes_read_{bytes_read} | ||
{ } | ||
|
||
exec_action_type type() const { return type_; } | ||
system::error_code error() const { return ec_; } | ||
std::size_t bytes_read() const { return bytes_read_; } | ||
}; | ||
|
||
class exec_fsm { | ||
int resume_point_{0}; | ||
multiplexer* mpx_{nullptr}; | ||
std::shared_ptr<multiplexer::elem> elem_; | ||
|
||
static bool is_cancellation(asio::cancellation_type_t type) | ||
{ | ||
return !!( | ||
type & (asio::cancellation_type_t::total | asio::cancellation_type_t::partial | | ||
asio::cancellation_type_t::terminal)); | ||
} | ||
|
||
exec_action resume_impl(bool connection_is_open, asio::cancellation_type_t cancel_state) | ||
{ | ||
switch (resume_point_) { | ||
BOOST_REDIS_CORO_INITIAL | ||
|
||
// Check whether the user wants to wait for the connection to | ||
// be established. | ||
if (elem_->get_request().get_config().cancel_if_not_connected && !connection_is_open) { | ||
BOOST_REDIS_YIELD(resume_point_, 1, exec_action_type::immediate) | ||
return system::error_code(error::not_connected); | ||
} | ||
|
||
// No more immediate errors. Set up the supported cancellation types. | ||
// This is required to get partial and total cancellations. | ||
// This is a potentially allocating operation, so do it as late as we can. | ||
BOOST_REDIS_YIELD(resume_point_, 2, exec_action_type::setup_cancellation) | ||
|
||
// Add the request to the multiplexer | ||
mpx_->add(elem_); | ||
|
||
// Notify the writer task that there is work to do. If the task is not | ||
// listening (e.g. it's already writing or the connection is not healthy), | ||
// this is a no-op. Since this is sync, no cancellation can happen here. | ||
BOOST_REDIS_YIELD(resume_point_, 3, exec_action_type::write) | ||
|
||
while (true) { | ||
// Wait until we get notified. This will return once the request completes, | ||
// or upon any kind of cancellation | ||
BOOST_REDIS_YIELD(resume_point_, 4, exec_action_type::wait_for_response) | ||
|
||
// If the request has completed (with error or not), we're done | ||
if (elem_->is_done()) { | ||
return exec_action{elem_->get_error(), elem_->get_read_size()}; | ||
} | ||
|
||
// If we're cancelled, try to remove the request from the queue. This will only | ||
// succeed if the request is waiting (wasn't written yet) | ||
if (is_cancellation(cancel_state) && mpx_->remove(elem_)) { | ||
return exec_action{asio::error::operation_aborted}; | ||
} | ||
|
||
// If we hit a terminal cancellation, tear down the connection. | ||
// Otherwise, go back to waiting. | ||
// TODO: we could likely do better here and mark the request as cancelled, removing | ||
// the done callback and the adapter. But this requires further exploration | ||
if (!!(cancel_state & asio::cancellation_type_t::terminal)) { | ||
BOOST_REDIS_YIELD(resume_point_, 5, exec_action_type::cancel_run) | ||
return exec_action{asio::error::operation_aborted}; | ||
} | ||
} | ||
} | ||
|
||
// We should never get here | ||
BOOST_ASSERT(false); | ||
return exec_action{system::error_code()}; | ||
} | ||
|
||
public: | ||
exec_fsm(multiplexer& mpx, std::shared_ptr<multiplexer::elem> elem) noexcept | ||
: mpx_(&mpx) | ||
, elem_(std::move(elem)) | ||
{ } | ||
|
||
exec_action resume(bool connection_is_open, asio::cancellation_type_t cancel_state) | ||
{ | ||
// When completing, we should deallocate any temporary storage we acquired | ||
// for the operation before invoking the final handler. | ||
// This intercepts the returned action to implement this. | ||
auto act = resume_impl(connection_is_open, cancel_state); | ||
if (act.type() == exec_action_type::done) | ||
elem_.reset(); | ||
return act; | ||
} | ||
}; | ||
|
||
} // namespace boost::redis::detail | ||
|
||
#endif // BOOST_REDIS_CONNECTOR_HPP |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,7 @@ auto multiplexer::elem::notify_error(system::error_code ec) noexcept -> void | |
ec_ = ec; | ||
} | ||
|
||
done_(); | ||
notify_done(); | ||
} | ||
|
||
auto multiplexer::elem::commit_response(std::size_t read_size) -> void | ||
|
@@ -119,6 +119,7 @@ std::pair<tribool, std::size_t> multiplexer::consume_next(system::error_code& ec | |
|
||
if (ec) { | ||
reqs_.front()->notify_error(ec); | ||
reqs_.pop_front(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting, this is probably fixing a bug. |
||
return std::make_pair(std::make_optional(false), 0); | ||
} | ||
|
||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the naming here could be more descriptive
notify_writer
for example.