SERVER-65292 NetworkInterfaceTL::shutdown may race with the destructor

2 files changed, 59 insertions, 10 deletions

diff --git a/src/mongo/executor/network_interface_tl.cpp b/src/mongo/executor/network_interface_tl.cpp
index 555b3677de4..34dea94ee5c 100644
--- a/src/mongo/executor/network_interface_tl.cpp
+++ b/src/mongo/executor/network_interface_tl.cpp
@@ -201,8 +201,11 @@ NetworkInterfaceTL::NetworkInterfaceTL(std::string instanceName,
 }
 
 NetworkInterfaceTL::~NetworkInterfaceTL() {
-    if (!inShutdown()) {
-        shutdown();
+    shutdown();
+
+    {
+        stdx::unique_lock lk(_stateMutex);
+        _stoppedCV.wait(lk, [&] { return _state == kStopped; });
     }
 
     // Because we quick exit on shutdown, these invariants are usually checked only in ASAN builds
@@ -246,7 +249,9 @@ void NetworkInterfaceTL::startup() {
         _run();
     });
 
-    invariant(_state.swap(kStarted) == kDefault);
+    stdx::lock_guard stateLock(_stateMutex);
+    invariant(_state == kDefault, "Network interface has already started");
+    _state = kStarted;
 }
 
 void NetworkInterfaceTL::_run() {
@@ -267,11 +272,34 @@ void NetworkInterfaceTL::_run() {
 }
 
 void NetworkInterfaceTL::shutdown() {
-    if (_state.swap(kStopped) != kStarted)
-        return;
+
+    {
+        stdx::lock_guard lk(_stateMutex);
+        switch (_state) {
+            case kDefault:
+                _state = kStopped;
+                _stoppedCV.notify_one();
+                return;
+            case kStarted:
+                _state = kStopping;
+                break;
+            case kStopping:
+            case kStopped:
+                LOGV2_INFO(6529201,
+                           "Network interface redundant shutdown",
+                           "state"_attr = toString(_state));
+                return;
+        }
+    }
 
     LOGV2_DEBUG(22594, 2, "Shutting down network interface.");
 
+    const ScopeGuard finallySetStopped = [&] {
+        stdx::lock_guard lk(_stateMutex);
+        _state = kStopped;
+        _stoppedCV.notify_one();
+    };
+
     // Cancel any remaining commands. Any attempt to register new commands will throw.
     auto inProgress = [&] {
         stdx::lock_guard lk(_inProgressMutex);
@@ -300,7 +328,8 @@ void NetworkInterfaceTL::shutdown() {
 }
 
 bool NetworkInterfaceTL::inShutdown() const {
-    return _state.load() == kStopped;
+    stdx::lock_guard lk(_stateMutex);
+    return _state == kStopping || _state == kStopped;
 }
 
 void NetworkInterfaceTL::waitForWork() {
diff --git a/src/mongo/executor/network_interface_tl.h b/src/mongo/executor/network_interface_tl.h
index 158e4b0a679..8564413806a 100644
--- a/src/mongo/executor/network_interface_tl.h
+++ b/src/mongo/executor/network_interface_tl.h
@@ -353,14 +353,34 @@ private:
 
     std::unique_ptr<rpc::EgressMetadataHook> _metadataHook;
 
-    // We start in kDefault, transition to kStarted after startup() is complete and enter kStopped
-    // at the first call to shutdown()
-    enum State : int {
+    // We start in kDefault, transition to kStarted after a call to startup completes.
+    // Enter kStopping at the first call to shutdown and transition to kStopped
+    // when the call completes.
+    enum State {
         kDefault,
         kStarted,
+        kStopping,
         kStopped,
     };
-    AtomicWord<State> _state;
+
+    friend StringData toString(State s) {
+        return std::array{
+            "Default"_sd,
+            "Started"_sd,
+            "Stopping"_sd,
+            "Stopped"_sd,
+        }
+            .at(s);
+    }
+
+    // This condition variable is dedicated to block a thread calling this class
+    // destructor, strictly when another thread is performing the network
+    // interface shutdown which depends on the _ioThread termination and may
+    // take an undeterministic amount of time to return.
+    mutable stdx::mutex _stateMutex;  // NOLINT
+    stdx::condition_variable _stoppedCV;
+    State _state;
+
     stdx::thread _ioThread;
 
     Mutex _inProgressMutex =