fix race on disconnect

between the time we check for `is_open` and the actual disconnect, the
socket could already have been disconnected by another thread (TOCTOU).
Furthermore, the disconnects can race causing a segfault or similar
issue in the asio's internals.

include/Client.h

@@ -18,6 +18,7 @@
 
 #pragma once
 
+#include <atomic>
 #include <chrono>
 #include <memory>
 #include <optional>
@@ -68,8 +69,11 @@ public:
     std::string GetCarPositionRaw(int Ident);
     void SetUDPAddr(const ip::udp::endpoint& Addr) { mUDPAddress = Addr; }
     void SetTCPSock(ip::tcp::socket&& CSock) { mSocket = std::move(CSock); }
-    void Disconnect(std::string_view Reason);
+    // Returns true only for the thread that actually performs socket shutdown/close.
-    bool IsDisconnected() const { return !mSocket.is_open(); }
+    [[nodiscard]] bool Disconnect(std::string_view Reason);
+    bool IsDisconnected() const {
+        return mDisconnectState.load(std::memory_order_acquire) != EDisconnectState::Connected;
+    }
     // locks
     void DeleteCar(int Ident);
     [[nodiscard]] const std::unordered_map<std::string, std::string>& GetIdentifiers() const { return mIdentifiers; }
@@ -106,6 +110,12 @@ public:
     [[nodiscard]] const std::vector<uint8_t>& GetMagic() const { return mMagic; }
 
 private:
+    enum class EDisconnectState {
+        Connected,
+        Disconnecting,
+        Disconnected
+    };
+
     void InsertVehicle(int ID, const std::string& Data);
 
     TServer& mServer;
@@ -121,6 +131,8 @@ private:
     TSetOfVehicleData mVehicleData;
     SparseArray<std::string> mVehiclePosition;
     std::string mName = "Unknown Client";
+    // Once disconnect starts, this client is terminal and its socket must be treated as dead.
+    std::atomic<EDisconnectState> mDisconnectState { EDisconnectState::Connected };
     ip::tcp::socket mSocket;
     ip::udp::endpoint mUDPAddress {};
     int mUnicycleID = -1;

src/Client.cpp

@@ -76,7 +76,13 @@ std::string TClient::GetCarPositionRaw(int Ident) {
     }
 }
 
-void TClient::Disconnect(std::string_view Reason) {
+bool TClient::Disconnect(std::string_view Reason) {
+    // Do not remove this guard: concurrent close() on the same socket can crash in Asio internals.
+    EDisconnectState Expected = EDisconnectState::Connected;
+    if (!mDisconnectState.compare_exchange_strong(Expected, EDisconnectState::Disconnecting, std::memory_order_acq_rel, std::memory_order_acquire)) {
+        return false;
+    }
+
     beammp_debugf("Disconnecting client {} for reason: {}", GetID(), Reason);
     boost::system::error_code ec;
     if (mSocket.is_open()) {
@@ -91,6 +97,9 @@ void TClient::Disconnect(std::string_view Reason) {
     } else {
         beammp_debug("Socket is already closed.");
     }
+    // Terminal state: this client must not perform TCP IO after this point.
+    mDisconnectState.store(EDisconnectState::Disconnected, std::memory_order_release);
+    return true;
 }
 
 void TClient::SetCarPosition(int Ident, const std::string& Data) {

src/TNetwork.cpp

@@ -697,9 +697,8 @@ void TNetwork::DisconnectClient(const std::weak_ptr<TClient>& c, const std::stri
 }
 
 void TNetwork::DisconnectClient(TClient& c, const std::string& R) {
-    if (c.IsDisconnected())
+    // Keep this unconditional; TClient::Disconnect() is the single-winner guard.
-        return;
+    (void)c.Disconnect(R);
-    c.Disconnect(R);
 }
 
 void TNetwork::Looper(const std::weak_ptr<TClient>& c) {
@@ -740,7 +739,7 @@ void TNetwork::Looper(const std::weak_ptr<TClient>& c) {
 
 void TNetwork::TCPClient(const std::weak_ptr<TClient>& c) {
     // TODO: the c.expired() might cause issues here, remove if you end up here with your debugger
-    if (c.expired() || !c.lock()->GetTCPSock().is_open()) {
+    if (c.expired() || c.lock()->IsDisconnected()) {
         mServer.RemoveClient(c);
         return;
     }

src/TPPSMonitor.cpp

@@ -76,7 +76,7 @@ void TPPSMonitor::operator()() {
             return true;
         });
         for (auto& ClientToKick : TimedOutClients) {
-            ClientToKick->Disconnect("Timeout");
+            Network().DisconnectClient(*ClientToKick, "Timeout");
         }
         TimedOutClients.clear();
         if (C == 0 || mInternalPPS == 0) {

src/TServer.cpp

@@ -224,7 +224,7 @@ void TServer::GlobalParser(const std::weak_ptr<TClient>& Client, std::vector<uin
     case 'p':
         if (!Network.Respond(*LockedClient, StringToVector("p"), false)) {
             // failed to send
-            LockedClient->Disconnect("Failed to send ping");
+            Network.DisconnectClient(*LockedClient, "Failed to send ping");
         } else {
             Network.UpdatePlayer(*LockedClient);
         }