From 2fae4242b9e8256da203639895d1ecd3fe8e2794 Mon Sep 17 00:00:00 2001
From: ADAM David Alan Martin <adam.martin@10gen.com>
Date: Wed, 16 Nov 2016 14:08:01 -0500
Subject: SERVER-27048 Fix recursive lock issue in transport.

The LegacySession teardown code has a race where promoted weak
pointers would be the last owners of a type which needs to hold
a lock in destruction.  That same lock is held by the LegacySession
teardown code, thus leading to a deadlock or detectable recursive
locking situation.
---
 src/mongo/transport/transport_layer_legacy.cpp | 51 ++++++++++++++++++++------
 src/mongo/transport/transport_layer_legacy.h   |  8 +++-
 2 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/src/mongo/transport/transport_layer_legacy.cpp b/src/mongo/transport/transport_layer_legacy.cpp
index 789b41529fa..a3b9c7306b0 100644
--- a/src/mongo/transport/transport_layer_legacy.cpp
+++ b/src/mongo/transport/transport_layer_legacy.cpp
@@ -30,6 +30,8 @@
 
 #include "mongo/platform/basic.h"
 
+#include <algorithm>
+#include <iterator>
 #include <memory>
 
 #include "mongo/transport/transport_layer_legacy.h"
@@ -47,6 +49,14 @@
 
 namespace mongo {
 namespace transport {
+namespace {
+struct lock_weak {
+    template <typename T>
+    std::shared_ptr<T> operator()(const std::weak_ptr<T>& p) const {
+        return p.lock();
+    }
+};
+}  // namespace
 
 TransportLayerLegacy::ListenerLegacy::ListenerLegacy(const TransportLayerLegacy::Options& opts,
                                                      NewConnectionCb callback)
@@ -221,23 +231,40 @@ void TransportLayerLegacy::_closeConnection(Connection* conn) {
     Listener::globalTicketHolder.release();
 }
 
+// Capture all of the weak pointers behind the lock, to delay their expiry until we leave the
+// locking context. This function requires proof of locking, by passing the lock guard.
+auto TransportLayerLegacy::lockAllSessions(const stdx::unique_lock<stdx::mutex>&) const
+    -> std::vector<LegacySessionHandle> {
+    using std::begin;
+    using std::end;
+    std::vector<std::shared_ptr<LegacySession>> result;
+    std::transform(begin(_sessions), end(_sessions), std::back_inserter(result), lock_weak());
+    // Skip expired weak pointers.
+    result.erase(std::remove(begin(result), end(result), nullptr), end(result));
+    return result;
+}
+
 void TransportLayerLegacy::endAllSessions(Session::TagMask tags) {
     log() << "legacy transport layer closing all connections";
     {
-        stdx::lock_guard<stdx::mutex> lk(_sessionsMutex);
-        for (auto&& it : _sessions) {
-
-            // Attempt to make our weak_ptr into a shared_ptr
-            auto session = it.lock();
-            if (session) {
-                if (session->getTags() & tags) {
-                    log() << "Skip closing connection for connection # "
-                          << session->conn()->connectionId;
-                } else {
-                    _closeConnection(session->conn());
-                }
+        stdx::unique_lock<stdx::mutex> lk(_sessionsMutex);
+        // We want to capture the shared_ptrs to our sessions in a way which lets us destroy them
+        // outside of the lock.
+        const auto sessions = lockAllSessions(lk);
+
+        for (auto&& session : sessions) {
+            if (session->getTags() & tags) {
+                log() << "Skip closing connection for connection # "
+                      << session->conn()->connectionId;
+            } else {
+                _closeConnection(session->conn());
             }
         }
+        // TODO(SERVER-27069): Revamp this lock to not cover the loop. This unlock was put here
+        // specifically to minimize risk, just before the release of 3.4. The risk is that we would
+        // be in the loop without the lock, which most of our testing didn't do. We must unlock
+        // manually here, because the `sessions` vector must be destroyed *outside* of the lock.
+        lk.unlock();
     }
 }
 
diff --git a/src/mongo/transport/transport_layer_legacy.h b/src/mongo/transport/transport_layer_legacy.h
index dab651bb86e..6ef7b73a7c1 100644
--- a/src/mongo/transport/transport_layer_legacy.h
+++ b/src/mongo/transport/transport_layer_legacy.h
@@ -28,6 +28,8 @@
 
 #pragma once
 
+#include <vector>
+
 #include "mongo/stdx/list.h"
 #include "mongo/stdx/memory.h"
 #include "mongo/stdx/mutex.h"
@@ -101,6 +103,8 @@ private:
     using NewConnectionCb = stdx::function<void(std::unique_ptr<AbstractMessagingPort>)>;
     using WorkHandle = stdx::function<Status(AbstractMessagingPort*)>;
 
+    std::vector<LegacySessionHandle> lockAllSessions(const stdx::unique_lock<stdx::mutex>&) const;
+
     /**
      * Connection object, to associate Sessions with AbstractMessagingPorts.
      */
@@ -127,8 +131,8 @@ private:
     public:
         ~LegacySession();
 
-        static std::shared_ptr<LegacySession> create(std::unique_ptr<AbstractMessagingPort> amp,
-                                                     TransportLayerLegacy* tl);
+        static LegacySessionHandle create(std::unique_ptr<AbstractMessagingPort> amp,
+                                          TransportLayerLegacy* tl);
 
         TransportLayer* getTransportLayer() const override {
             return _tl;
-- 
cgit v1.2.1