Skip to content
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

Inconsistency in cancellation logic when throw_if_cancelled_ = false #1574

Open
dragon-dreamer opened this issue Dec 22, 2024 · 2 comments
Open

Comments

@dragon-dreamer
Copy link

dragon-dreamer commented Dec 22, 2024

I have the following code example which demonstrates the inconsistency (boost 1.87, MSVC2022, Win10). What I do here is basically schedule two coroutines in a parallel group, and then expect the second one to cancel the first one. I call throw_if_cancelled(false) because I do not want any exceptions to occur, but instead, I want an error code if the timer is cancelled.

The first one (expect_that_this_function_is_cancelled) schedules an infinite timer. When the second coro completes, the timer is indeed cancelled (I see "Timer cancelled" printed out). Then, I check the cancellation state of the first coroutine, and it indicates, that the coroutine was indeed cancelled (I see "This coro is cancelled: 1").

However, when I schedule the same timer for the second time, it is no longer cancelled, even though it inherits the cancellation state which indicated that the parent coroutine was cancelled.

It means that the timer will be cancelled if the cancellation state switches to cancelled only when the timer is already initiated. However, if the timer is not yet scheduled, it will disregard the parent coro cancellation state (even if it is already cancelled), and wait forever.

The same issue is present in other primitives (e.g. channels).

Is this behavior by design? What is the right way to overcome this issue?

#include <boost/asio/io_context.hpp>
#include <boost/asio/steady_timer.hpp>
#include <boost/asio/awaitable.hpp>
#include <boost/asio/use_awaitable.hpp>
#include <boost/asio/co_spawn.hpp>
#include <boost/asio/detached.hpp>
#include <boost/asio/this_coro.hpp>
#include <boost/asio/as_tuple.hpp>
#include <boost/asio/experimental/awaitable_operators.hpp>

#include <iostream>

boost::asio::awaitable<void> expect_that_this_function_is_cancelled(
	boost::asio::steady_timer& timer_to_be_cancelled)
{
	co_await boost::asio::this_coro::throw_if_cancelled(false);

	std::cerr << "Waiting for a timer to be cancelled\n";
	co_await timer_to_be_cancelled.async_wait(boost::asio::as_tuple(boost::asio::use_awaitable));
	std::cerr << "Timer cancelled\n";

	std::cerr << "This coro is cancelled: " <<
		((co_await boost::asio::this_coro::cancellation_state).cancelled()
			!= boost::asio::cancellation_type::none) << '\n';

	std::cerr << "Waiting for a timer to be cancelled second time\n";
	co_await timer_to_be_cancelled.async_wait(boost::asio::as_tuple(boost::asio::use_awaitable));
        // This line is never printed, because the previous line waits forever
	std::cerr << "Timer cancelled 2\n";
}

boost::asio::awaitable<void> this_will_cancel(
	boost::asio::steady_timer& delayed_cancellation_timer)
{
	co_await boost::asio::this_coro::throw_if_cancelled(false);

	co_await delayed_cancellation_timer.async_wait(boost::asio::use_awaitable);
	std::cerr << "Cancelling another coroutine...\n";
}

int main()
{
	boost::asio::io_context ctx;

	boost::asio::steady_timer timer_to_be_cancelled(ctx,
		std::chrono::steady_clock::time_point::max());
	boost::asio::steady_timer delayed_cancellation_timer(ctx,
		std::chrono::seconds(5));

	using namespace boost::asio::experimental::awaitable_operators;
	boost::asio::co_spawn(ctx,
		(expect_that_this_function_is_cancelled(timer_to_be_cancelled)
			|| this_will_cancel(delayed_cancellation_timer)),
		boost::asio::detached);

	ctx.run();
}
@dragon-dreamer dragon-dreamer changed the title Race condition in cancellation logic Inconsistency in cancellation logic when throw_if_cancelled_ = false Dec 22, 2024
@klemens-morgenstern
Copy link
Contributor

That is not inconsistent but expecred behaviour. You need to check the state before initializatoon manually if you don't want to use the throw on cancelled default.

@dragon-dreamer
Copy link
Author

I am fine to close this as designed, but to me, this behavior still looks inconsistent.

I disable exceptions by calling throw_if_cancelled, and instead of throwing, I expect the async_wait call to always return an error code when it's been cancelled. It does return the correct error code if the coro is cancelled when async_wait has been already scheduled, but hangs when it's being scheduled after the coro was cancelled. This is not consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants