[COMMIT seastar master] shared_future: test for dangling tasks

[Commit Bot]

From: Piotr Dulikowski <pio...@scylladb.com>
Committer: Piotr Dulikowski <pio...@scylladb.com>
Branch: master

shared_future: test for dangling tasks

The previous version of the PR was reverted in scylladb/seastar#1937 due
to a slight change in how shared_future works internally: when a
shared_future was created from a not-yet-resolved future, it would
schedule a task to run after the future gets resolved. In case when, at
the end of the program, there are some unsatisfied shared_promises
(which are essentially promise + shared_future pairs) and they get
destroyed, the promises would be broken and shared futures' internal
tasks would get scheduled for execution. However, as nobody waits for
the tasks, it could happen that the program exits before they have a
chance to run. This is harmless by itself but causes leak detectors to
complain (example: LeakSanitizer in ScyllaDB's `schema_registry_test`
test).

The new version does not have this problem and schedules the task in
cases when the master implementation does - i.e. get_future must be
called, so the task is indirectly waited on. Add a test which verifies
that the internal task is scheduled at appropriate moments.

---
diff --git a/include/seastar/core/shared_future.hh b/include/seastar/core/shared_future.hh
--- a/include/seastar/core/shared_future.hh
+++ b/include/seastar/core/shared_future.hh
@@ -239,6 +239,11 @@ private:
bool failed() const noexcept {
return _original_future.failed();
}
+
+ // Used only in tests (see shared_future_tester in futures_test.cc)
+ bool has_scheduled_task() const noexcept {
+ return _keepaliver != nullptr;
+ }
};
/// \endcond
lw_shared_ptr<shared_state> _state;
@@ -296,6 +301,10 @@ public:
bool valid() const noexcept {
return bool(_state);
}
+
+ /// \cond internal
+ friend class shared_future_tester;
+ /// \endcond
};

/// \brief Like \ref promise except that its counterpart is \ref shared_future instead of \ref future
diff --git a/tests/unit/futures_test.cc b/tests/unit/futures_test.cc
--- a/tests/unit/futures_test.cc
+++ b/tests/unit/futures_test.cc
@@ -1561,6 +1561,55 @@ SEASTAR_THREAD_TEST_CASE(test_shared_promise_with_outstanding_future_is_immediat
BOOST_REQUIRE_THROW(f2.get(), std::runtime_error);
}

+namespace seastar {
+class shared_future_tester {
+public:
+ template <typename... T>
+ static bool has_scheduled_task(const shared_future<T...>& f) noexcept {
+ return f._state->has_scheduled_task();
+ }
+};
+}
+
+SEASTAR_THREAD_TEST_CASE(test_shared_future_task_scheduled_only_if_there_are_waiting_futures) {
+ {
+ // Case 1: promise is eventually satisfied, get_future is not called
+ promise<> pr1;
+ shared_future<> f1(pr1.get_future());
+ BOOST_REQUIRE(!shared_future_tester::has_scheduled_task(f1));
+
+ pr1.set_value();
+ // get_future was not called, so no task should have been scheduled
+ BOOST_REQUIRE(!shared_future_tester::has_scheduled_task(f1));
+ }
+
+ {
+ // Case 2: promise is eventually satisfied, get_future was called
+ promise<> pr2;
+ shared_future<> f2(pr2.get_future());
+ auto f2f = f2.get_future();
+
+ // get_future was called, so the task is scheduled to happen after promise is resolved
+ BOOST_REQUIRE(shared_future_tester::has_scheduled_task(f2));
+
+ pr2.set_value();
+ f2f.get();
+ // f2f is resolved by shared future's task, so it must have run
+ BOOST_REQUIRE(!shared_future_tester::has_scheduled_task(f2));
+ }
+
+ {
+ // Case 3: shared future is ready from the start
+ shared_future<> f3(make_ready_future<>());
+ BOOST_REQUIRE(!shared_future_tester::has_scheduled_task(f3));
+
+ auto f3f = f3.get_future();
+ // Calling get_future on a ready shared future does not schedule the task
+ BOOST_REQUIRE(!shared_future_tester::has_scheduled_task(f3));
+ BOOST_REQUIRE(f3f.available());
+ }
+}
+
SEASTAR_TEST_CASE(test_when_all_succeed_tuples) {
return seastar::when_all_succeed(
make_ready_future<>(),