From 2a51a3dcf9cf5d93d5d65011e7f96729d19c332d Mon Sep 17 00:00:00 2001
From: Xuehan Xu <xxhdx1985126@163.com>
Date: Fri, 6 Mar 2020 18:55:07 +0800
Subject: [PATCH] crimson: decouple mgr client reconnect and connect reset
 handling

As of now, the following invocation sequence triggers deadlock when
closing crimson-osd's connection with mgr:
  ProtocolV2::dispatch_reset() --> crimson::mgr::Client::ms_handle_reset
--> crimson::mgr::Client::reconnect --> crimson::net::SocketConnection::close
--> crimson::net::Protocol::close()

In the above invocation sequence, ProtocalV2::dispatch_reset() enters the gate
"pending_dispatch" the leaving of which would wait for the complete of crimson::\
net::Protocal::close() which further wait for the complete of the gate's close().

This commit decouples this waiting chain.

Signed-off-by: Xuehan Xu <xxhdx1985126@163.com>
---
 src/crimson/mgr/client.cc | 37 ++++++++++++++++++++++---------------
 src/crimson/mgr/client.h  |  6 +++---
 2 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/src/crimson/mgr/client.cc b/src/crimson/mgr/client.cc
index 9a9c78580c83d..0cebc1c1e44bb 100644
--- a/src/crimson/mgr/client.cc
+++ b/src/crimson/mgr/client.cc
@@ -26,7 +26,7 @@ Client::Client(crimson::net::Messenger& msgr,
                  WithStats& with_stats)
   : msgr{msgr},
     with_stats{with_stats},
-    report_timer{[this] {report();}}
+    tick_timer{[this] {tick();}}
 {}
 
 seastar::future<> Client::start()
@@ -61,10 +61,9 @@ seastar::future<> Client::ms_dispatch(crimson::net::Connection* conn,
 seastar::future<> Client::ms_handle_reset(crimson::net::ConnectionRef c)
 {
   if (conn == c) {
-    return reconnect();
-  } else {
-    return seastar::now();
+    conn = nullptr;
   }
+  return seastar::now();
 }
 
 seastar::future<> Client::reconnect()
@@ -101,22 +100,30 @@ seastar::future<> Client::handle_mgr_conf(crimson::net::Connection* conn,
                                           Ref<MMgrConfigure> m)
 {
   logger().info("{} {}", __func__, *m);
-  report_period = std::chrono::seconds{m->stats_period};
-  if (report_period.count() && !report_timer.armed() ) {
-    report();
+  tick_period = std::chrono::seconds{m->stats_period};
+  if (tick_period.count() && !tick_timer.armed() ) {
+    tick();
   }
   return seastar::now();
 }
 
-void Client::report()
+void Client::tick()
 {
-  (void) seastar::with_gate(gate, [this] {
-    auto pg_stats = with_stats.get_stats();
-    return conn->send(std::move(pg_stats)).finally([this] {
-      if (report_period.count()) {
-        report_timer.arm(report_period);
-      }
-    });
+  (void) seastar::with_gate(gate, [=] {
+    if (conn) {
+      auto pg_stats = with_stats.get_stats();
+      return conn->send(std::move(pg_stats)).finally([this] {
+	if (tick_period.count()) {
+	  tick_timer.arm(tick_period);
+	}
+      });
+    } else {
+      return reconnect().finally([this] {
+	if (tick_period.count()) {
+	  tick_timer.arm(tick_period);
+	}
+      });;
+    }
   });
 }
 
diff --git a/src/crimson/mgr/client.h b/src/crimson/mgr/client.h
index fa4ae39df348f..a82b73bc1f81b 100644
--- a/src/crimson/mgr/client.h
+++ b/src/crimson/mgr/client.h
@@ -45,15 +45,15 @@ private:
   seastar::future<> handle_mgr_conf(crimson::net::Connection* conn,
 				    Ref<MMgrConfigure> m);
   seastar::future<> reconnect();
-  void report();
+  void tick();
 
 private:
   MgrMap mgrmap;
   crimson::net::Messenger& msgr;
   WithStats& with_stats;
   crimson::net::ConnectionRef conn;
-  std::chrono::seconds report_period{0};
-  seastar::timer<seastar::lowres_clock> report_timer;
+  std::chrono::seconds tick_period{0};
+  seastar::timer<seastar::lowres_clock> tick_timer;
   seastar::gate gate;
 };
 
-- 
2.39.5