From 6b25d8375df28c8764c67bfcd09607495b7d17d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Blin?= <sebastien.blin@savoirfairelinux.com> Date: Wed, 14 Apr 2021 14:52:40 -0400 Subject: [PATCH] jamiaccount: fix startSync race condition. When receiving a new DeviceAnnoucement the other side will try to get the device certificate. However we can't be sure that the certificate is on the DHT. So sync will generally fail the first time. This patch introduces a new callback to startSync only when the certificate of the device is present on the DHT. Change-Id: If2334051b1eea07d5036f50f095e5d64ef3ba660 --- src/jamidht/jamiaccount.cpp | 64 +++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp index a74361ed60..7f559b5e3a 100644 --- a/src/jamidht/jamiaccount.cpp +++ b/src/jamidht/jamiaccount.cpp @@ -2393,6 +2393,35 @@ JamiAccount::doRegister_() *currentDhtStatus = newStatus; setRegistrationState(state); }; + context.identityAnnouncedCb = [this](bool ok) { + if (!ok) + return; + accountManager_->startSync([this](const std::shared_ptr<dht::crypto::Certificate>& crt) { + if (!crt) + return; + auto deviceId = crt->getId().toString(); + if (accountManager_->getInfo()->deviceId == deviceId) + return; + + std::lock_guard<std::mutex> lk(connManagerMtx_); + if (!connectionManager_) + connectionManager_ = std::make_unique<ConnectionManager>(*this); + auto channelName = "sync://" + deviceId; + if (connectionManager_->isConnecting(crt->getId(), channelName)) { + JAMI_INFO("[Account %s] Already connecting to %s", + getAccountID().c_str(), + deviceId.c_str()); + return; + } + connectionManager_->connectDevice(crt, + channelName, + [this](std::shared_ptr<ChannelSocket> socket, + const DeviceId& deviceId) { + if (socket) + syncWith(deviceId.toString(), socket); + }); + }); + }; setRegistrationState(RegistrationState::TRYING); dht_->run(dhtPortUsed(), config, std::move(context)); @@ -2401,34 +2430,7 @@ JamiAccount::doRegister_() dht_->bootstrap(bootstrap); accountManager_->setDht(dht_); - accountManager_->startSync([this](const std::shared_ptr<dht::crypto::Certificate>& crt) { - if (!crt) - return; - auto deviceId = crt->getId().toString(); - if (accountManager_->getInfo()->deviceId == deviceId) - return; - - std::lock_guard<std::mutex> lk(connManagerMtx_); - if (!connectionManager_) - return; - - auto channelName = "sync://" + deviceId; - if (connectionManager_->isConnecting(crt->getId(), channelName)) { - JAMI_INFO("[Account %s] Already connecting to %s", - getAccountID().c_str(), - deviceId.c_str()); - return; - } - connectionManager_->connectDevice(crt, - channelName, - [this](std::shared_ptr<ChannelSocket> socket, - const DeviceId& deviceId) { - if (socket) - syncWith(deviceId.toString(), socket); - }); - }); - // Init connection manager std::unique_lock<std::mutex> lkCM(connManagerMtx_); if (!connectionManager_) connectionManager_ = std::make_unique<ConnectionManager>(*this); @@ -2440,14 +2442,14 @@ JamiAccount::doRegister_() deviceId, [this, &accept](const std::shared_ptr<dht::crypto::Certificate>& cert) { dht::InfoHash peer_account_id; auto res = accountManager_->onPeerCertificate(cert, - dhtPublicInCalls_, - peer_account_id); + dhtPublicInCalls_, + peer_account_id); if (res) JAMI_INFO("Accepting ICE request from account %s", - peer_account_id.toString().c_str()); + peer_account_id.toString().c_str()); else JAMI_INFO("Discarding ICE request from account %s", - peer_account_id.toString().c_str()); + peer_account_id.toString().c_str()); accept.set_value(res); }); fut.wait(); -- GitLab