From 3ffd819e5e2dfe31edd008872d6784b9f648d40c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adrien=20B=C3=A9raud?= <adrien.beraud@savoirfairelinux.com>
Date: Tue, 2 May 2023 18:50:22 -0400
Subject: [PATCH] ConnectionManager: get new channel id atomically

* lock connectCbsMtx_ only once when reading from and writing to pendingCbs_
* only lookup deviceId in pendingCbs_ once, instead of at least twice
* removed not working MAX_TENTATIVE mechanism

Change-Id: Ic4027dcb2f5a1a32e8f73e0f38c0f53451eabf19
---
 src/connectivity/connectionmanager.cpp | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/src/connectivity/connectionmanager.cpp b/src/connectivity/connectionmanager.cpp
index 08c8d61793..88d4bc3baa 100644
--- a/src/connectivity/connectionmanager.cpp
+++ b/src/connectivity/connectionmanager.cpp
@@ -35,7 +35,6 @@
 #include <set>
 
 static constexpr std::chrono::seconds DHT_MSG_TIMEOUT {30};
-static constexpr int MAX_TENTATIVES {100};
 using ValueIdDist = std::uniform_int_distribution<dht::Value::Id>;
 using CallbackId = std::pair<jami::DeviceId, dht::Value::Id>;
 
@@ -503,23 +502,18 @@ ConnectionManager::Impl::connectDevice(const std::shared_ptr<dht::crypto::Certif
             cb(nullptr, deviceId);
             return;
         }
-        dht::Value::Id vid;
-        auto tentatives = 0;
-        do {
-            vid = ValueIdDist(1, JAMI_ID_MAX_VAL)(sthis->account.rand);
-            --tentatives;
-        } while (sthis->getPendingCallbacks(deviceId, vid).size() != 0
-                 && tentatives != MAX_TENTATIVES);
-        if (tentatives == MAX_TENTATIVES) {
-            JAMI_ERR("Couldn't get a current random channel number");
-            cb(nullptr, deviceId);
-            return;
-        }
+        dht::Value::Id vid = ValueIdDist(1, JAMI_ID_MAX_VAL)(sthis->account.rand);
         auto isConnectingToDevice = false;
         {
             std::lock_guard<std::mutex> lk(sthis->connectCbsMtx_);
-            // Check if already connecting
             auto pendingsIt = sthis->pendingCbs_.find(deviceId);
+            if (pendingsIt != sthis->pendingCbs_.end()) {
+                const auto& pendings = pendingsIt->second;
+                while (std::find_if(pendings.begin(), pendings.end(), [&](const auto& it){ return it.vid == vid; }) != pendings.end()) {
+                    vid = ValueIdDist(1, JAMI_ID_MAX_VAL)(sthis->account.rand);
+                }
+            }
+            // Check if already connecting
             isConnectingToDevice = pendingsIt != sthis->pendingCbs_.end();
             // Save current request for sendChannelRequest.
             // Note: do not return here, cause we can be in a state where first
-- 
GitLab