[COMMIT seastar master] util/closeable: add move ctor for deferred_stop

[Commit Bot]

From: Kefu Chai <tcha...@gmail.com>
Committer: Avi Kivity <a...@scylladb.com>
Branch: master

util/closeable: add move ctor for deferred_stop

before this change, if we move from a deferred_stop, when the original
instance is destructed, the obj referenced by it is always stopped if
the deferred_stop is not canceled. and the new instance of deferred_stop
also stops the referenced object when it is destroyed. but developer
would expect the obj is stopped only once, and should be stopped by the
new deferred_stop instance. this behavior is surprising when developer
intentially capture the deferred_stop instance by move and store it in
a local variable.

in this change,

* a move ctor is added for deferred_stop, so that the original
instance is always canceled.
* a test is added accordingly.

Signed-off-by: Kefu Chai <tcha...@gmail.com>

---
diff --git a/include/seastar/util/closeable.hh b/include/seastar/util/closeable.hh
--- a/include/seastar/util/closeable.hh
+++ b/include/seastar/util/closeable.hh
@@ -61,6 +61,19 @@ public:
/// Construct an object that will auto-close \c obj when destroyed.
/// \tparam obj the object to auto-close.
deferred_close(Object& obj) noexcept : _obj(obj) {}
+ /// Moves the \c deferred_close into a new one, and
+ /// the old one is canceled.
+ deferred_close(deferred_close&& x) noexcept : _obj(x._obj), _closed(std::exchange(x._closed, true)) {}
+ deferred_close(const deferred_close&) = delete;
+ /// Move-assign another \ref deferred_close.
+ /// The current \ref deferred_close is closed before being assigned.
+ /// And the other one's state is transferred to the current one.
+ deferred_close& operator=(deferred_close&& x) noexcept {
+ do_close();
+ _obj = x._obj;
+ _closed = std::exchange(x._closed, true);
+ return *this;
+ }
/// Destruct the deferred_close object and auto-close \c obj.
~deferred_close() {
do_close();
@@ -122,6 +135,19 @@ public:
/// Construct an object that will auto-stop \c obj when destroyed.
/// \tparam obj the object to auto-stop.
deferred_stop(Object& obj) noexcept : _obj(obj) {}
+ /// Moves the \c deferred_stop into a new one, and
+ /// the old one is canceled.
+ deferred_stop(deferred_stop&& x) noexcept : _obj(x._obj), _stopped(std::exchange(x._stopped, true)) {}
+ deferred_stop(const deferred_stop&) = delete;
+ /// Move-assign another \ref deferred_stop.
+ /// The current \ref deferred_stop is stopped before being assigned.
+ /// And the other one's state is transferred to the current one.
+ deferred_stop& operator=(deferred_stop&& x) noexcept {
+ do_stop();
+ _obj = x._obj;
+ _stopped = std::exchange(x._stopped, true);
+ return *this;
+ }
/// Destruct the deferred_stop object and auto-stop \c obj.
~deferred_stop() {
do_stop();
diff --git a/tests/unit/closeable_test.cc b/tests/unit/closeable_test.cc
--- a/tests/unit/closeable_test.cc
+++ b/tests/unit/closeable_test.cc
@@ -56,6 +56,22 @@ SEASTAR_TEST_CASE(deferred_close_test) {
});
}

+SEASTAR_TEST_CASE(move_deferred_close_test) {
+ return do_with(gate(), [] (gate& g) {
+ return async([&] {
+ auto close_gate = make_shared(deferred_close(g));
+ // g.close() should not be called when deferred_close is moved away
+ BOOST_REQUIRE(!g.is_closed());
+ }).then([&] {
+ // Before this test is exercised, gate::close() would run into a
+ // assert failure when leaving previous continuation, if gate::close()
+ // is called twice, so this test only verifies the behavior with the
+ // release build.
+ BOOST_REQUIRE(g.is_closed());
+ });
+ });
+}
+
SEASTAR_TEST_CASE(close_now_test) {
return do_with(gate(), 0, 42, [] (gate& g, int& count, int& expected) {
return async([&] {
@@ -176,6 +192,18 @@ SEASTAR_TEST_CASE(deferred_stop_test) {
});
}

+SEASTAR_TEST_CASE(move_deferred_stop_test) {
+ return do_with(count_stops(), [] (count_stops& cs) {
+ return async([&] {
+ auto stop = make_shared(deferred_stop(cs));
+ }).then([&] {
+ // cs.stop() should be called once and only once
+ // when stop is destroyed
+ BOOST_REQUIRE_EQUAL(cs.stopped(), 1);
+ });
+ });
+}
+
SEASTAR_TEST_CASE(stop_now_test) {
return do_with(count_stops(), [] (count_stops& cs) {
return async([&] {