SERVER-22391 Handle race in connpool hostTimeout

If a new request comes in at the same moment a hostTimeout triggers we
can race in shutdown (and hit an invariant).  An additional check fixes
the misbehavior.

2 files changed, 48 insertions, 0 deletions

diff --git a/src/mongo/executor/connection_pool.cpp b/src/mongo/executor/connection_pool.cpp
index 1d41cded111..fb88bef8577 100644
--- a/src/mongo/executor/connection_pool.cpp
+++ b/src/mongo/executor/connection_pool.cpp
@@ -533,6 +533,25 @@ void ConnectionPool::SpecificPool::spawnConnections(stdx::unique_lock<stdx::mute
 void ConnectionPool::SpecificPool::shutdown() {
     stdx::unique_lock<stdx::mutex> lk(_parent->_mutex);
 
+    // We're racing:
+    //
+    // Thread A (this thread)
+    //   * Fired the shutdown timer
+    //   * Came into shutdown() and blocked
+    //
+    // Thread B (some new consumer)
+    //   * Requested a new connection
+    //   * Beat thread A to the mutex
+    //   * Cancelled timer (but thread A already made it in)
+    //   * Set state to running
+    //   * released the mutex
+    //
+    // So we end up in shutdown, but with kRunning.  If we're here we raced and
+    // we should just bail.
+    if (_state == State::kRunning) {
+        return;
+    }
+
     _state = State::kInShutdown;
 
     // If we have processing connections, wait for them to finish or timeout
diff --git a/src/mongo/executor/connection_pool_asio_integration_test.cpp b/src/mongo/executor/connection_pool_asio_integration_test.cpp
index 4c5f2424aa4..3a8460e1f99 100644
--- a/src/mongo/executor/connection_pool_asio_integration_test.cpp
+++ b/src/mongo/executor/connection_pool_asio_integration_test.cpp
@@ -41,6 +41,7 @@
 #include "mongo/unittest/integration_test.h"
 #include "mongo/unittest/unittest.h"
 #include "mongo/util/scopeguard.h"
+#include "mongo/util/time_support.h"
 
 namespace mongo {
 namespace executor {
@@ -120,6 +121,34 @@ TEST(ConnectionPoolASIO, TestPing) {
     ASSERT_LTE(MyNetworkConnectionHook::count(), 10);
 }
 
+/**
+ * This test verifies a fix for SERVER-22391, where we raced if a new request
+ * came in at the same time a host timeout was triggering.
+ */
+TEST(ConnectionPoolASIO, TestHostTimeoutRace) {
+    auto fixture = unittest::getFixtureConnectionString();
+
+    NetworkInterfaceASIO::Options options;
+    options.streamFactory = stdx::make_unique<AsyncStreamFactory>();
+    options.connectionPoolOptions.hostTimeout = Milliseconds(1);
+    options.timerFactory = stdx::make_unique<AsyncTimerFactoryASIO>();
+    NetworkInterfaceASIO net{std::move(options)};
+
+    net.startup();
+    auto guard = MakeGuard([&] { net.shutdown(); });
+
+    for (int i = 0; i < 1000; i++) {
+        Deferred<StatusWith<RemoteCommandResponse>> deferred;
+        net.startCommand(
+            makeCallbackHandle(),
+            RemoteCommandRequest{fixture.getServers()[0], "admin", BSON("ping" << 1), BSONObj()},
+            [&](StatusWith<RemoteCommandResponse> resp) { deferred.emplace(std::move(resp)); });
+
+        ASSERT_OK(deferred.get().getStatus());
+        sleepmillis(1);
+    }
+}
+
 }  // namespace
 }  // namespace executor
 }  // namespace mongo