From 8cd00200669fa5b7632faa447ba206c3847e2879 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adrien=20B=C3=A9raud?= <adrien.beraud@savoirfairelinux.com>
Date: Wed, 23 Oct 2024 15:42:09 -0400
Subject: [PATCH] ConnectionManager: reset ID of the connecting channel on
 retry

Avoid re-using the same ID from the previous connection attempt,
which prevents various issues, including the new connectionInfo being
destroyed just after the retry by the async shutdown callback
from the previous connection.

Change-Id: I9c9017f7d0b3107dd8311b5066ba558d8c87b053
---
 src/connectionmanager.cpp | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/src/connectionmanager.cpp b/src/connectionmanager.cpp
index 5f70622..a341b95 100644
--- a/src/connectionmanager.cpp
+++ b/src/connectionmanager.cpp
@@ -181,7 +181,8 @@ struct DeviceInfo {
         return info.empty() && connecting.empty() && waiting.empty();
     }
 
-    dht::Value::Id newId(std::mt19937_64& rand) const {
+    dht::Value::Id newId(std::mt19937_64& rand, std::mutex& mtx) const {
+        std::lock_guard lkr(mtx);
         ValueIdDist dist(1, ID_MAX_VAL);
         dht::Value::Id id;
         do {
@@ -557,7 +558,7 @@ class ConnectionManager::Impl : public std::enable_shared_from_this<ConnectionMa
     void addNewMultiplexedSocket(const std::weak_ptr<DeviceInfo>& dinfo, const DeviceId& deviceId, const dht::Value::Id& vid, const std::shared_ptr<ConnectionInfo>& info);
     void onPeerResponse(PeerConnectionRequest&& req);
     void onDhtConnected(const dht::crypto::PublicKey& devicePk);
-    void retryOnError(const std::shared_ptr<DeviceInfo>& deviceInfo);
+    void retryOnError(const std::shared_ptr<DeviceInfo>& deviceInfo, std::unique_lock<std::mutex>& lk);
 
     const std::shared_future<tls::DhParams> dhParams() const;
     tls::CertificateStore& certStore() const { return *config_->certStore; }
@@ -626,7 +627,7 @@ class ConnectionManager::Impl : public std::enable_shared_from_this<ConnectionMa
     std::shared_ptr<ConnectionManager::Config> config_;
     std::unique_ptr<std::thread> ioContextRunner_;
 
-    mutable std::mutex randMtx_;
+    mutable std::mutex randMutex_;
     mutable std::mt19937_64 rand_;
 
     iOSConnectedCallback iOSConnectedCb_ {};
@@ -925,11 +926,7 @@ ConnectionManager::Impl::connectDevice(const std::shared_ptr<dht::crypto::Certif
             di->cert = cert;
         }
 
-        dht::Value::Id vid;
-        {
-            std::lock_guard lkr(sthis->randMtx_);
-            vid = di->newId(sthis->rand_);
-        }
+        dht::Value::Id vid = di->newId(sthis->rand_, sthis->randMutex_);
 
         // Check if already connecting
         auto isConnectingToDevice = di->isConnecting();
@@ -1580,14 +1577,20 @@ ConnectionManager::Impl::addNewMultiplexedSocket(const std::weak_ptr<DeviceInfo>
                             sthis->infos_.removeDeviceInfo(deviceInfo->deviceId);
                     }
                     lkc.unlock();
-                    lkd.unlock();
-
-                    for (auto& op : ops)
-                        op.cb(nullptr, deviceInfo->deviceId);
                     if (retry) {
-                        if (auto sthis = w.lock())
-                            sthis->retryOnError(deviceInfo);
+                        if (auto sthis = w.lock()) {
+                            // Reset state and ID of the connecting channel
+                            if (auto connecting = deviceInfo->connecting.extract(vid)) {
+                                dht::Value::Id vid = deviceInfo->newId(sthis->rand_, sthis->randMutex_);
+                                deviceInfo->waiting[vid] = std::move(connecting.mapped());
+                            }
+                            sthis->retryOnError(deviceInfo, lkd);
+                        }
                     }
+                    if (lkd)
+                        lkd.unlock();
+                    for (auto& op : ops)
+                        op.cb(nullptr, deviceInfo->deviceId);
                 }
             }
         });
@@ -1595,9 +1598,8 @@ ConnectionManager::Impl::addNewMultiplexedSocket(const std::weak_ptr<DeviceInfo>
 }
 
 void
-ConnectionManager::Impl::retryOnError(const std::shared_ptr<DeviceInfo>& deviceInfo)
+ConnectionManager::Impl::retryOnError(const std::shared_ptr<DeviceInfo>& deviceInfo, std::unique_lock<std::mutex>& lk)
 {
-    std::unique_lock<std::mutex> lk(deviceInfo->mutex_);
     if (not deviceInfo->isConnecting())
         return;
     if (auto i = deviceInfo->getConnectedInfo()) {