[PATCH] conditional-variable: aline when() and wait() behaviour in case of a predicate throwing an exception

[Gleb Natapov]

Currently if predicate provided to wait() throws an exception it is
returned as a result of the wait(), but in case of when() the exception
is causes process termination since it is thrown in non except context.

The patch makes when() behave the same way as wait().

diff --git a/include/seastar/core/condition-variable.hh b/include/seastar/core/condition-variable.hh
index 100264861b..ba78872d1f 100644
--- a/include/seastar/core/condition-variable.hh
+++ b/include/seastar/core/condition-variable.hh
@@ -21,6 +21,7 @@

#pragma once

+#include "seastar/core/future.hh"
#ifndef SEASTAR_MODULE
#include <boost/intrusive/list.hpp>
#include <chrono>
@@ -161,22 +162,30 @@ class condition_variable {
, _func(std::move(func))
{}
void signal() noexcept override {
- if (_func()) {
- Base::signal();
- } else {
- // must re-enter waiter queue
- // this maintains "wait" version
- // semantics of moving to back of queue
- // if predicate fails
- Base::_cv->add_waiter(*this);
+ try {
+ if (_func()) {
+ Base::signal();
+ } else {
+ // must re-enter waiter queue
+ // this maintains "wait" version
+ // semantics of moving to back of queue
+ // if predicate fails
+ Base::_cv->add_waiter(*this);
+ }
+ } catch(...) {
+ Base::set_exception(std::current_exception());
}
}
auto operator co_await() {
- if (_func()) {
- return ::seastar::internal::awaiter<false, void>(make_ready_future<>());
- } else {
- Base::_cv->check_and_consume_signal(); // clear out any signal state
- return Base::operator co_await();
+ try {
+ if (_func()) {
+ return ::seastar::internal::awaiter<false, void>(make_ready_future<>());
+ } else {
+ Base::_cv->check_and_consume_signal(); // clear out any signal state
+ return Base::operator co_await();
+ }
+ } catch (...) {
+ return ::seastar::internal::awaiter<false, void>(make_exception_future(std::current_exception()));
}
}
};
diff --git a/tests/unit/condition_variable_test.cc b/tests/unit/condition_variable_test.cc
index 180102afeb..17203d432f 100644
--- a/tests/unit/condition_variable_test.cc
+++ b/tests/unit/condition_variable_test.cc
@@ -363,3 +363,34 @@ SEASTAR_TEST_CASE(test_condition_variable_when_timeout) {

co_await std::move(f);
}
+
+// Check that exception from a predicate is propogated to the waiter
+SEASTAR_THREAD_TEST_CASE(test_condition_variable_wait_predicate_throws) {
+ condition_variable cv;
+
+ auto f = cv.wait([i = 1] () mutable {
+ if (i == 0) {
+ throw std::runtime_error("Predicate error");
+ }
+ i--;
+ return false;
+ });
+ cv.signal();
+ BOOST_REQUIRE_THROW(f.get(), std::runtime_error);
+}
+
+// Check that exception from a predicate is propogated to the waiter
+SEASTAR_TEST_CASE(test_condition_variable_when_predicate_throws) {
+ condition_variable cv;
+
+ (void)sleep(100ms).then([&] { cv.signal(); });
+
+ BOOST_REQUIRE_THROW(co_await
+ cv.when([i = 1] () mutable {
+ if (i == 0) {
+ throw std::runtime_error("Predicate error");
+ }
+ i--;
+ return false;
+ }), std::runtime_error);
+}
--
Gleb.

[Gleb Natapov]

Ping.

--
Gleb.

[Benny Halevy]

LGTM