From: Radosław Zarzyński <rzarzyns@redhat.com>
Date: Fri, 17 Jun 2022 12:17:25 +0000 (+0200)
Subject: msg: fix deadlock when handling existing but closed v2 connection
X-Git-Tag: v17.2.4~1^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=052dac980b1d22aa75935fe7c92c6a6a856a1417;p=ceph.git

msg: fix deadlock when handling existing but closed v2 connection

The deadlock is illustrated best by the following snippet
provided by jianwei zhang who also made the problem analysis
(many thanks!).

```
thread-35
AsyncMessenger::shutdown_connections         hold             AsyncMessenger::lock            std::lock_guard l{lock}
AsyncConnection::stop                         wait                AsyncConnection::lock            lock.lock()

thread-3
ProtocolV2::handle_existing_connection        hold                AsyncConnection::lock            std::lock_guard<std::mutex> l(existing->lock)
AsyncMessenger::accept_conn                wait                AsyncMessenger::lock            std::lock_guard l{lock}
```

Fixes: https://tracker.ceph.com/issues/55355
Signed-off-by: Radosław Zarzyński <rzarzyns@redhat.com>
(cherry picked from commit a6fcb1ccbc44e60416eb5f4e2c7291afe3a9d44d)
---

diff --git a/src/msg/async/ProtocolV2.cc b/src/msg/async/ProtocolV2.cc
index a176fc2c808..40ebbf01633 100644
--- a/src/msg/async/ProtocolV2.cc
+++ b/src/msg/async/ProtocolV2.cc
@@ -2596,7 +2596,7 @@ CtPtr ProtocolV2::handle_reconnect(ceph::bufferlist &payload)
 CtPtr ProtocolV2::handle_existing_connection(const AsyncConnectionRef& existing) {
   ldout(cct, 20) << __func__ << " existing=" << existing << dendl;
 
-  std::lock_guard<std::mutex> l(existing->lock);
+  std::unique_lock<std::mutex> l(existing->lock);
 
   ProtocolV2 *exproto = dynamic_cast<ProtocolV2 *>(existing->protocol.get());
   if (!exproto) {
@@ -2607,6 +2607,7 @@ CtPtr ProtocolV2::handle_existing_connection(const AsyncConnectionRef& existing)
   if (exproto->state == CLOSED) {
     ldout(cct, 1) << __func__ << " existing " << existing << " already closed."
                   << dendl;
+    l.unlock();
     return send_server_ident();
   }
 
@@ -2636,6 +2637,7 @@ CtPtr ProtocolV2::handle_existing_connection(const AsyncConnectionRef& existing)
         << dendl;
     existing->protocol->stop();
     existing->dispatch_queue->queue_reset(existing.get());
+    l.unlock();
     return send_server_ident();
   }