[COMMIT seastar master] gate: add move ctor and move assignment operator for gate

2 views
Skip to first unread message

Commit Bot

<bot@cloudius-systems.com>
unread,
Oct 4, 2022, 6:18:33 AM10/4/22
to seastar-dev@googlegroups.com, Kefu Chai
From: Kefu Chai <tcha...@gmail.com>
Committer: Pavel Emelyanov <xe...@scylladb.com>
Branch: master

gate: add move ctor and move assignment operator for gate

we have a use case where a std::vector<> is used for holding a number of
domain specific "io channel" instances. the type of element in the
vector is plain io channel, not a smart pointer, as all members
variables in io channel have well defined move ctor and all of them are
noexcept. each of io channel uses a seastar::gate to ensure that all
the submitted requests are completed before the io channel is destroyed.
but when we want to increase the number of io channels in the
std::vector<>, there is a chance that the vector reallocates the
elements in it. if the io channel has its move ctor defined as noexcept,
the vector should be able to use it without calling the dtor to destroy
the old elements and calling copy ctor to create the new ones.

but the problem is, the request count tracked by seastar::gate
is left unchanged after its move ctor is called. so when the dtor of
moved-away io channel is called, the dtor of its gate would have
assertion failure, because of the "outstanding requests", despite
that they are already moved into the newly allocated gate.

in this change, the move ctor and move assignment operator are
added for gate, so that it can be moved without confusing its dtor.

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

Closes #1225

---
diff --git a/include/seastar/core/gate.hh b/include/seastar/core/gate.hh
--- a/include/seastar/core/gate.hh
+++ b/include/seastar/core/gate.hh
@@ -50,8 +50,16 @@ class gate {
public:
gate() = default;
gate(const gate&) = delete;
- gate(gate&&) = default;
- gate& operator=(gate&&) = default;
+ gate(gate&& x) noexcept
+ : _count(std::exchange(x._count, 0)), _stopped(std::exchange(x._stopped, std::nullopt)) {}
+ gate& operator=(gate&& x) noexcept {
+ if (this != &x) {
+ assert(!_count && "gate reassigned with outstanding requests");
+ _count = std::exchange(x._count, 0);
+ _stopped = std::exchange(x._stopped, std::nullopt);
+ }
+ return *this;
+ }
~gate() {
assert(!_count && "gate destroyed with outstanding requests");
}
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
@@ -246,6 +246,45 @@ SEASTAR_TEST_CASE(with_stoppable_exception_test) {
});
}

+SEASTAR_THREAD_TEST_CASE(move_open_gate_test) {
+ gate g1;
+ g1.enter();
+ // move an open gate
+ gate g2 = std::move(g1);
+ // the state in g1 should be moved into g2
+ BOOST_CHECK_EQUAL(g1.get_count(), 0);
+ BOOST_REQUIRE_EQUAL(g2.get_count(), 1);
+ g2.leave();
+ g2.close().get();
+ BOOST_CHECK(!g1.is_closed());
+ BOOST_CHECK(g2.is_closed());
+}
+
+SEASTAR_THREAD_TEST_CASE(move_closing_gate_test) {
+ gate g1;
+ g1.enter();
+ auto fut = g1.close();
+ // move a closing gate
+ gate g2 = std::move(g1);
+ BOOST_CHECK_EQUAL(g1.get_count(), 0);
+ BOOST_REQUIRE_EQUAL(g2.get_count(), 1);
+ g2.leave();
+ fut.get();
+ BOOST_CHECK(!g1.is_closed());
+ BOOST_CHECK(g2.is_closed());
+}
+
+SEASTAR_THREAD_TEST_CASE(move_closed_gate_test) {
+ gate g1;
+ g1.close().get();
+ // move a closed gate
+ gate g2 = std::move(g1);
+ BOOST_CHECK_EQUAL(g1.get_count(), 0);
+ BOOST_CHECK_EQUAL(g2.get_count(), 0);
+ BOOST_CHECK(!g1.is_closed());
+ BOOST_CHECK(g2.is_closed());
+}
+
SEASTAR_THREAD_TEST_CASE(gate_holder_basic_test) {
gate g;
auto gh = g.hold();
Reply all
Reply to author
Forward
0 new messages