msg/async: mark down local_connection before draining the stack.

`AsyncMessenger` has `local_connection` – an instance of
`AsyncConnection` used to send messages to itself (loop back).
Connections are handled by `EventCenter` in its dedicated
thread. When shutting down a messenger, it must be ensured
there is no task left in the `EventCenter`'s queue. Otherwise
a use-after-free can happen. That's the reason why `shutdown()`
of `AsyncMessenger` does `mark_down_all()` on connections
**before** draining the stack. The latter actually injects a task
into all `EventCenter` instances and waits for its completion
(synchronization barrier).

However, that's not the case for `local_connection`. Without
the patch it's marked down by the messenger's destructor
far **after** the synchronization barrier. This behavior is
dangerous when an implementation of `mark_down()` creates
a new task to be executed inside the boundaries of corresponding
`EventCenter` instance.

The fix unifies handling of `local_connection` with other
connections in the aspect of the shutdown phase.

Fixes: https://tracker.ceph.com/issues/43667
Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>

diff --git a/src/msg/async/AsyncMessenger.cc b/src/msg/async/AsyncMessenger.cc
index b7aa8241eabc20eb50cd82b1ed8c975dd8feda25..56263322ed1fd96f2cb5e97e88327f0001353d8b 100644 (file)
--- a/src/msg/async/AsyncMessenger.cc
+++ b/src/msg/async/AsyncMessenger.cc
@@ -314,7 +314,6 @@ AsyncMessenger::~AsyncMessenger()
 {
   delete reap_handler;
   ceph_assert(!did_bind); // either we didn't bind or we shut down the Processor
-  local_connection->mark_down();
   for (auto &&p : processors)
     delete p;
 }
@@ -348,6 +347,7 @@ int AsyncMessenger::shutdown()
   mark_down_all();
   // break ref cycles on the loopback connection
   local_connection->set_priv(NULL);
+  local_connection->mark_down();
   did_bind = false;
   lock.lock();
   stop_cond.notify_all();