crimson/net: allow mark_down() inside ms_handle_reset()

Although it is not necessary to mark_down the connection in its
ms_handle_reset() event, but it can be more convenient to allow it.
And Heartbeat already encounters this assertion failure.

So move the assertion to close_clean() which will help identify problems
if we happen to make ms_handle_reset() wait for messenger shutdown.

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>

diff --git a/src/crimson/net/Protocol.cc b/src/crimson/net/Protocol.cc
index 8ae7bb1d3af080f8739d8783f49b3bf47b96b634..7eb952609ed00fbfc448c758dbeb02cc4a7b2176 100644 (file)
--- a/src/crimson/net/Protocol.cc
+++ b/src/crimson/net/Protocol.cc
@@ -45,7 +45,6 @@ void Protocol::close(bool dispatch_reset,
 {
   if (closed) {
     // already closing
-    assert(close_ready.valid());
     return;
   }
 
@@ -60,10 +59,8 @@ void Protocol::close(bool dispatch_reset,
 #endif
     };
 
-  // close_ready become valid only after state is state_t::closing
-  assert(!close_ready.valid());
-
   // atomic operations
+  closed = true;
   trigger_close();
   if (f_accept_new) {
     (*f_accept_new)();
@@ -71,7 +68,6 @@ void Protocol::close(bool dispatch_reset,
   if (socket) {
     socket->shutdown();
   }
-  closed = true;
   set_write_state(write_state_t::drop);
   auto gate_closed = pending_dispatch.close();
   auto reset_dispatched = seastar::futurize_apply([this, dispatch_reset] {
@@ -86,6 +82,7 @@ void Protocol::close(bool dispatch_reset,
   });
 
   // asynchronous operations
+  assert(!close_ready.valid());
   close_ready = seastar::when_all_succeed(
     std::move(gate_closed).finally([this] {
       if (socket) {

diff --git a/src/crimson/net/Protocol.h b/src/crimson/net/Protocol.h
index c5f8a5b728627ed42af2a62cc3b004e322028413..f70f4b0b291c3fc27c831987eac70a37121c2262 100644 (file)
--- a/src/crimson/net/Protocol.h
+++ b/src/crimson/net/Protocol.h
@@ -34,6 +34,9 @@ class Protocol {
   void close(bool dispatch_reset, std::optional<std::function<void()>> f_accept_new=std::nullopt);
   seastar::future<> close_clean(bool dispatch_reset) {
     close(dispatch_reset);
+    // it can happen if close_clean() is called inside Dispatcher::ms_handle_reset()
+    // which will otherwise result in deadlock
+    assert(close_ready.valid());
     return close_ready.get_future();
   }