diff --git a/src/account.cpp b/src/account.cpp index f52a3365726f3f2b67cfa708109a6230e7aae327..521e566a939cd85cf1bfc7756508640dbd5e6015 100644 --- a/src/account.cpp +++ b/src/account.cpp @@ -668,14 +668,6 @@ Account::getActiveAccountCodecInfoList(MediaType mediaType) const return accountCodecList; } -const IceTransportOptions -Account::getIceOptions() const noexcept -{ - IceTransportOptions opts; - opts.upnpEnable = getUPnPActive(); - return opts; -} - const std::string& Account::getUserAgentName() { diff --git a/src/account.h b/src/account.h index 40cbf6507e612783096e70b77f4fc217aa501c0e..0282713af431419c6cfac34a0709230f77fbd119 100644 --- a/src/account.h +++ b/src/account.h @@ -197,11 +197,6 @@ public: virtual std::map<std::string, std::string> getNearbyPeers() const { return {}; } - /** - * Store the local/public addresses used to register - */ - virtual void storeActiveIpAddress() {}; - /** * Return the status corresponding to the token. */ @@ -305,8 +300,6 @@ public: */ IpAddr getUPnPIpAddress() const; - virtual const IceTransportOptions getIceOptions() const noexcept; - /** * Random generator engine * Logical account state shall never rely on the state of the random generator. diff --git a/src/jamidht/connectionmanager.cpp b/src/jamidht/connectionmanager.cpp index 1ad0ff66c00b53e31967c3a772eb23d57e668c7f..ee210b8bfc541d093fe0cea5f6e30cbc5ca20c43 100644 --- a/src/jamidht/connectionmanager.cpp +++ b/src/jamidht/connectionmanager.cpp @@ -472,78 +472,87 @@ ConnectionManager::Impl::connectDevice(const std::shared_ptr<dht::crypto::Certif }; // If no socket exists, we need to initiate an ICE connection. - auto ice_config = sthis->account.getIceOptions(); - ice_config.tcpEnable = true; - ice_config.onInitDone = [w, - cbId, - deviceId = std::move(deviceId), - name = std::move(name), - cert = std::move(cert), - vid, - eraseInfo](bool ok) { - auto sthis = w.lock(); - if (!sthis) - return; - if (!ok) { - JAMI_ERR("Cannot initialize ICE session."); - for (const auto& pending : sthis->extractPendingCallbacks(deviceId)) - pending.cb(nullptr, deviceId); - runOnMainThread([eraseInfo = std::move(eraseInfo)] { eraseInfo(); }); - return; - } - - dht::ThreadPool::io().run( - [w = std::move(w), deviceId = std::move(deviceId), vid = std::move(vid)] { - if (auto sthis = w.lock()) - sthis->connectDeviceStartIce(deviceId, vid); - }); - }; - ice_config.onNegoDone = [w, - cbId, - deviceId = std::move(deviceId), - name = std::move(name), - cert = std::move(cert), - vid, - eraseInfo](bool ok) { + sthis->account.getIceOptions(std::move([w, + cbId, + deviceId = std::move(deviceId), + name = std::move(name), + cert = std::move(cert), + vid, + eraseInfo](auto ice_config) { auto sthis = w.lock(); if (!sthis) return; - if (!ok) { - JAMI_ERR("ICE negotiation failed."); - for (const auto& pending : sthis->extractPendingCallbacks(deviceId)) - pending.cb(nullptr, deviceId); - runOnMainThread([eraseInfo = std::move(eraseInfo)] { eraseInfo(); }); - return; - } + ice_config.tcpEnable = true; + ice_config.onInitDone = [w, + cbId, + deviceId = std::move(deviceId), + name = std::move(name), + cert = std::move(cert), + vid, + eraseInfo](bool ok) { + auto sthis = w.lock(); + if (!sthis) + return; + if (!ok) { + JAMI_ERR("Cannot initialize ICE session."); + for (const auto& pending : sthis->extractPendingCallbacks(deviceId)) + pending.cb(nullptr, deviceId); + runOnMainThread([eraseInfo = std::move(eraseInfo)] { eraseInfo(); }); + return; + } - dht::ThreadPool::io().run([w = std::move(w), - deviceId = std::move(deviceId), - name = std::move(name), - cert = std::move(cert), - vid = std::move(vid)] { + dht::ThreadPool::io().run( + [w = std::move(w), deviceId = std::move(deviceId), vid = std::move(vid)] { + if (auto sthis = w.lock()) + sthis->connectDeviceStartIce(deviceId, vid); + }); + }; + ice_config.onNegoDone = [w, + cbId, + deviceId = std::move(deviceId), + name = std::move(name), + cert = std::move(cert), + vid, + eraseInfo](bool ok) { auto sthis = w.lock(); if (!sthis) return; - sthis->connectDeviceOnNegoDone(deviceId, name, vid, cert); - }); - }; + if (!ok) { + JAMI_ERR("ICE negotiation failed."); + for (const auto& pending : sthis->extractPendingCallbacks(deviceId)) + pending.cb(nullptr, deviceId); + runOnMainThread([eraseInfo = std::move(eraseInfo)] { eraseInfo(); }); + return; + } - auto info = std::make_shared<ConnectionInfo>(); - { - std::lock_guard<std::mutex> lk(sthis->infosMtx_); - sthis->infos_[{deviceId, vid}] = info; - } - std::unique_lock<std::mutex> lk {info->mutex_}; - info->ice_ = Manager::instance().getIceTransportFactory().createUTransport( - sthis->account.getAccountID().c_str(), 1, false, ice_config); + dht::ThreadPool::io().run([w = std::move(w), + deviceId = std::move(deviceId), + name = std::move(name), + cert = std::move(cert), + vid = std::move(vid)] { + auto sthis = w.lock(); + if (!sthis) + return; + sthis->connectDeviceOnNegoDone(deviceId, name, vid, cert); + }); + }; - if (!info->ice_) { - JAMI_ERR("Cannot initialize ICE session."); - for (const auto& pending : sthis->extractPendingCallbacks(deviceId)) - pending.cb(nullptr, deviceId); - eraseInfo(); - return; - } + auto info = std::make_shared<ConnectionInfo>(); + { + std::lock_guard<std::mutex> lk(sthis->infosMtx_); + sthis->infos_[{deviceId, vid}] = info; + } + std::unique_lock<std::mutex> lk {info->mutex_}; + info->ice_ = Manager::instance().getIceTransportFactory().createUTransport( + sthis->account.getAccountID().c_str(), 1, false, ice_config); + + if (!info->ice_) { + JAMI_ERR("Cannot initialize ICE session."); + for (const auto& pending : sthis->extractPendingCallbacks(deviceId)) + pending.cb(nullptr, deviceId); + eraseInfo(); + } + })); }); } @@ -816,76 +825,80 @@ ConnectionManager::Impl::onDhtPeerRequest(const PeerConnectionRequest& req, return; } - // Note: used when the ice negotiation fails to erase - // all stored structures. - auto eraseInfo = [w = weak(), id = req.id, from = req.from] { - if (auto shared = w.lock()) { - std::lock_guard<std::mutex> lk(shared->infosMtx_); - shared->infos_.erase({from, id}); - } - }; - // Because the connection is accepted, create an ICE socket. - auto ice_config = account.getIceOptions(); - ice_config.tcpEnable = true; - ice_config.onInitDone = [w = weak(), req, eraseInfo](bool ok) { + account.getIceOptions(std::move([w=weak(), req](auto ice_config) { + auto deviceId = req.from.toString(); auto shared = w.lock(); if (!shared) return; - if (!ok) { - JAMI_ERR("Cannot initialize ICE session."); - if (shared->connReadyCb_) - shared->connReadyCb_(req.from, "", nullptr); - runOnMainThread([eraseInfo = std::move(eraseInfo)] { eraseInfo(); }); - return; - } + // Note: used when the ice negotiation fails to erase + // all stored structures. + auto eraseInfo = [w, id = req.id, from = req.from] { + if (auto shared = w.lock()) { + std::lock_guard<std::mutex> lk(shared->infosMtx_); + shared->infos_.erase({from, id}); + } + }; - dht::ThreadPool::io().run([w = std::move(w), req = std::move(req)] { + ice_config.tcpEnable = true; + ice_config.onInitDone = [w, req, eraseInfo](bool ok) { auto shared = w.lock(); if (!shared) return; - shared->onRequestStartIce(req); - }); - }; + if (!ok) { + JAMI_ERR("Cannot initialize ICE session."); + if (shared->connReadyCb_) + shared->connReadyCb_(req.from, "", nullptr); + runOnMainThread([eraseInfo = std::move(eraseInfo)] { eraseInfo(); }); + return; + } - ice_config.onNegoDone = [w = weak(), req, eraseInfo](bool ok) { - auto shared = w.lock(); - if (!shared) - return; - if (!ok) { - JAMI_ERR("ICE negotiation failed"); + dht::ThreadPool::io().run([w = std::move(w), req = std::move(req)] { + auto shared = w.lock(); + if (!shared) + return; + shared->onRequestStartIce(req); + }); + }; + + ice_config.onNegoDone = [w, req, eraseInfo](bool ok) { + auto shared = w.lock(); + if (!shared) + return; + if (!ok) { + JAMI_ERR("ICE negotiation failed"); + if (shared->connReadyCb_) + shared->connReadyCb_(req.from, "", nullptr); + runOnMainThread([eraseInfo = std::move(eraseInfo)] { eraseInfo(); }); + return; + } + + dht::ThreadPool::io().run([w = std::move(w), req = std::move(req)] { + if (auto shared = w.lock()) + shared->onRequestOnNegoDone(req); + }); + }; + + // Negotiate a new ICE socket + auto info = std::make_shared<ConnectionInfo>(); + { + std::lock_guard<std::mutex> lk(shared->infosMtx_); + shared->infos_[{req.from, req.id}] = info; + } + JAMI_INFO("[Account:%s] accepting connection from %s", + shared->account.getAccountID().c_str(), + deviceId.c_str()); + std::unique_lock<std::mutex> lk {info->mutex_}; + info->ice_ = Manager::instance() + .getIceTransportFactory() + .createUTransport(shared->account.getAccountID().c_str(), 1, true, ice_config); + if (not info->ice_) { + JAMI_ERR("Cannot initialize ICE session."); if (shared->connReadyCb_) shared->connReadyCb_(req.from, "", nullptr); - runOnMainThread([eraseInfo = std::move(eraseInfo)] { eraseInfo(); }); - return; + eraseInfo(); } - - dht::ThreadPool::io().run([w = std::move(w), req = std::move(req)] { - if (auto shared = w.lock()) - shared->onRequestOnNegoDone(req); - }); - }; - - // Negotiate a new ICE socket - auto info = std::make_shared<ConnectionInfo>(); - { - std::lock_guard<std::mutex> lk(infosMtx_); - infos_[{req.from, req.id}] = info; - } - JAMI_INFO("[Account:%s] accepting connection from %s", - account.getAccountID().c_str(), - deviceId.c_str()); - std::unique_lock<std::mutex> lk {info->mutex_}; - info->ice_ = Manager::instance() - .getIceTransportFactory() - .createUTransport(account.getAccountID().c_str(), 1, true, ice_config); - if (not info->ice_) { - JAMI_ERR("Cannot initialize ICE session."); - if (connReadyCb_) - connReadyCb_(req.from, "", nullptr); - eraseInfo(); - return; - } + })); } void diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp index 91d25b1b89705b18ddf24c5ea4ace03ac00205d7..35c4955f8c89e608848c579e57c7190841d8c7f9 100644 --- a/src/jamidht/jamiaccount.cpp +++ b/src/jamidht/jamiaccount.cpp @@ -718,68 +718,69 @@ JamiAccount::onConnectedOutgoingCall(const std::shared_ptr<SIPCall>& call, return; JAMI_DBG("[call:%s] outgoing call connected to %s", call->getCallId().c_str(), to_id.c_str()); - auto opts = getIceOptions(); - opts.onInitDone = [w = weak(), target = std::move(target), call](bool ok) { - if (!ok) { - JAMI_ERR("ICE medias are not initialized"); - return; - } - auto shared = w.lock(); - if (!shared or !call) - return; - - const auto localAddress = ip_utils::getInterfaceAddr(shared->getLocalInterface(), - target.getFamily()); - - IpAddr addrSdp = shared->getPublishedSameasLocal() - ? localAddress - : shared->getPublishedIpAddress(target.getFamily()); + getIceOptions(std::move([=](auto opts) { + opts.onInitDone = [w = weak(), target = std::move(target), call](bool ok) { + if (!ok) { + JAMI_ERR("ICE medias are not initialized"); + return; + } + auto shared = w.lock(); + if (!shared or !call) + return; - // fallback on local address - if (not addrSdp) - addrSdp = localAddress; + const auto localAddress = ip_utils::getInterfaceAddr(shared->getLocalInterface(), + target.getFamily()); - // Initialize the session using ULAW as default codec in case of early media - // The session should be ready to receive media once the first INVITE is sent, before - // the session initialization is completed - if (!getSystemCodecContainer()->searchCodecByName("PCMA", jami::MEDIA_AUDIO)) - JAMI_WARN("Could not instantiate codec for early media"); + IpAddr addrSdp = shared->getPublishedSameasLocal() + ? localAddress + : shared->getPublishedIpAddress(target.getFamily()); - // Building the local SDP offer - auto& sdp = call->getSDP(); + // fallback on local address + if (not addrSdp) + addrSdp = localAddress; - sdp.setPublishedIP(addrSdp); + // Initialize the session using ULAW as default codec in case of early media + // The session should be ready to receive media once the first INVITE is sent, before + // the session initialization is completed + if (!getSystemCodecContainer()->searchCodecByName("PCMA", jami::MEDIA_AUDIO)) + JAMI_WARN("Could not instantiate codec for early media"); - auto mediaAttrList = call->getMediaAttributeList(); + // Building the local SDP offer + auto& sdp = call->getSDP(); - if (mediaAttrList.empty()) { - JAMI_ERR("Call [%s] has no media. Abort!", call->getCallId().c_str()); - return; - } + sdp.setPublishedIP(addrSdp); - if (not sdp.createOffer(mediaAttrList)) { - JAMI_ERR("Could not send outgoing INVITE request for new call"); - return; - } + auto mediaAttrList = call->getMediaAttributeList(); - // Note: pj_ice_strans_create can call onComplete in the same thread - // This means that iceMutex_ in IceTransport can be locked when onInitDone is called - // So, we need to run the call creation in the main thread - // Also, we do not directly call SIPStartCall before receiving onInitDone, because - // there is an inside waitForInitialization that can block the thread. - runOnMainThread([w = std::move(w), target = std::move(target), call = std::move(call)] { - auto shared = w.lock(); - if (!shared) + if (mediaAttrList.empty()) { + JAMI_ERR("Call [%s] has no media. Abort!", call->getCallId().c_str()); return; + } - if (not shared->SIPStartCall(*call, target)) { + if (not sdp.createOffer(mediaAttrList)) { JAMI_ERR("Could not send outgoing INVITE request for new call"); + return; } - }); - }; - call->setIPToIP(true); - call->setPeerNumber(to_id); - call->initIceMediaTransport(true, std::move(opts)); + + // Note: pj_ice_strans_create can call onComplete in the same thread + // This means that iceMutex_ in IceTransport can be locked when onInitDone is called + // So, we need to run the call creation in the main thread + // Also, we do not directly call SIPStartCall before receiving onInitDone, because + // there is an inside waitForInitialization that can block the thread. + runOnMainThread([w = std::move(w), target = std::move(target), call = std::move(call)] { + auto shared = w.lock(); + if (!shared) + return; + + if (not shared->SIPStartCall(*call, target)) { + JAMI_ERR("Could not send outgoing INVITE request for new call"); + } + }); + }; + call->setIPToIP(true); + call->setPeerNumber(to_id); + call->initIceMediaTransport(true, std::move(opts)); + })); } bool @@ -2695,46 +2696,47 @@ JamiAccount::incomingCall(dht::IceCandidates&& msg, return; } auto callId = call->getCallId(); - auto iceOptions = getIceOptions(); - iceOptions.onNegoDone = [callId, w = weak()](bool) { - runOnMainThread([callId, w]() { - if (auto shared = w.lock()) - shared->checkPendingCall(callId); - }); - }; - auto ice = createIceTransport(("sip:" + call->getCallId()).c_str(), - ICE_COMPONENTS, - false, - iceOptions); - iceOptions.tcpEnable = true; - auto ice_tcp = createIceTransport(("sip:" + call->getCallId()).c_str(), - ICE_COMPONENTS, - true, - iceOptions); + getIceOptions(std::move([=](auto iceOptions) { + iceOptions.onNegoDone = [callId, w = weak()](bool) { + runOnMainThread([callId, w]() { + if (auto shared = w.lock()) + shared->checkPendingCall(callId); + }); + }; + auto ice = createIceTransport(("sip:" + call->getCallId()).c_str(), + ICE_COMPONENTS, + false, + iceOptions); + iceOptions.tcpEnable = true; + auto ice_tcp = createIceTransport(("sip:" + call->getCallId()).c_str(), + ICE_COMPONENTS, + true, + iceOptions); + + std::weak_ptr<SIPCall> wcall = call; + Manager::instance().addTask([account = shared(), wcall, ice, ice_tcp, msg, from_cert, from] { + auto call = wcall.lock(); + + // call aborted? + if (not call) + return false; - std::weak_ptr<SIPCall> wcall = call; - Manager::instance().addTask([account = shared(), wcall, ice, ice_tcp, msg, from_cert, from] { - auto call = wcall.lock(); + if (ice->isFailed()) { + JAMI_ERR("[call:%s] ice init failed", call->getCallId().c_str()); + call->onFailure(EIO); + return false; + } - // call aborted? - if (not call) - return false; + // Loop until ICE transport is initialized. + // Note: we suppose that ICE init routine has a an internal timeout (bounded in time) + // and we let upper layers decide when the call shall be aborted (our first check upper). + if ((not ice->isInitialized()) || (ice_tcp && !ice_tcp->isInitialized())) + return true; - if (ice->isFailed()) { - JAMI_ERR("[call:%s] ice init failed", call->getCallId().c_str()); - call->onFailure(EIO); + account->replyToIncomingIceMsg(call, ice, ice_tcp, msg, from_cert, from); return false; - } - - // Loop until ICE transport is initialized. - // Note: we suppose that ICE init routine has a an internal timeout (bounded in time) - // and we let upper layers decide when the call shall be aborted (our first check upper). - if ((not ice->isInitialized()) || (ice_tcp && !ice_tcp->isInitialized())) - return true; - - account->replyToIncomingIceMsg(call, ice, ice_tcp, msg, from_cert, from); - return false; - }); + }); + })); } void @@ -3666,28 +3668,30 @@ JamiAccount::onIsComposing(const std::string& peer, bool isWriting) } } -const IceTransportOptions -JamiAccount::getIceOptions() const noexcept -{ - auto opts = SIPAccountBase::getIceOptions(); - auto publishedAddr = getPublishedIpAddress(); - - if (publishedAddr) { - auto interfaceAddr = ip_utils::getInterfaceAddr(getLocalInterface(), - publishedAddr.getFamily()); - if (interfaceAddr) { - opts.accountLocalAddr = interfaceAddr; - opts.accountPublicAddr = publishedAddr; +void +JamiAccount::getIceOptions(std::function<void(IceTransportOptions)> cb) noexcept +{ + storeActiveIpAddress([this, cb=std::move(cb)] { + auto opts = SIPAccountBase::getIceOptions(); + auto publishedAddr = getPublishedIpAddress(); + + if (publishedAddr) { + auto interfaceAddr = ip_utils::getInterfaceAddr(getLocalInterface(), + publishedAddr.getFamily()); + if (interfaceAddr) { + opts.accountLocalAddr = interfaceAddr; + opts.accountPublicAddr = publishedAddr; + } } - } - - return opts; + if (cb) + cb(std::move(opts)); + }); } void -JamiAccount::storeActiveIpAddress() +JamiAccount::storeActiveIpAddress(std::function<void()>&& cb) { - dht_->getPublicAddress([this](std::vector<dht::SockAddr>&& results) { + dht_->getPublicAddress([this, cb = std::move(cb)](std::vector<dht::SockAddr>&& results) { bool hasIpv4 {false}, hasIpv6 {false}; for (auto& result : results) { auto family = result.getFamily(); @@ -3714,6 +3718,8 @@ JamiAccount::storeActiveIpAddress() if (hasIpv4 and hasIpv6) break; } + if (cb) + cb(); }); } diff --git a/src/jamidht/jamiaccount.h b/src/jamidht/jamiaccount.h index 33225c6e90c481eb05bb3b5be04071b73c3e82ed..c27e62dacebf2891e2695abcbb0cfe64af94d2de 100644 --- a/src/jamidht/jamiaccount.h +++ b/src/jamidht/jamiaccount.h @@ -435,12 +435,12 @@ public: /** * Store the local/public addresses used to register */ - void storeActiveIpAddress() override; + void storeActiveIpAddress(std::function<void()>&& cb = {}); /** * Create and return ICE options. */ - const IceTransportOptions getIceOptions() const noexcept override; + void getIceOptions(std::function<void(IceTransportOptions)> cb) noexcept; #ifdef DRING_TESTABLE ConnectionManager& connectionManager() { return *connectionManager_; } diff --git a/src/sip/sipaccountbase.cpp b/src/sip/sipaccountbase.cpp index c3374d1f4f85b5b1adf11204631c868ad7695bfd..1017a494ac30a502e13c6629cc572a1cb674f68a 100644 --- a/src/sip/sipaccountbase.cpp +++ b/src/sip/sipaccountbase.cpp @@ -468,7 +468,8 @@ SIPAccountBase::generateVideoPort() const const IceTransportOptions SIPAccountBase::getIceOptions() const noexcept { - auto opts = Account::getIceOptions(); // Local copy of global account ICE settings + IceTransportOptions opts; + opts.upnpEnable = getUPnPActive(); if (stunEnabled_) opts.stunServers.emplace_back(StunServerInfo().setUri(stunServer_)); diff --git a/src/sip/sipaccountbase.h b/src/sip/sipaccountbase.h index 4dc37f677feb21b330692fd71bb4a51cc714824d..447f142f1b1f2b56145e653d7507a118f89df609 100644 --- a/src/sip/sipaccountbase.h +++ b/src/sip/sipaccountbase.h @@ -243,7 +243,7 @@ public: void setStunServer(const std::string& srv) { stunServer_ = srv; } - const IceTransportOptions getIceOptions() const noexcept override; + const IceTransportOptions getIceOptions() const noexcept; virtual void sendTextMessage(const std::string& to, const std::map<std::string, std::string>& payloads, diff --git a/src/upnp/upnp_context.cpp b/src/upnp/upnp_context.cpp index d4c325a62010463215bb72a107bd5c61e11a1bdd..21ab80726af24f405dd1945b8311aa0ba9cd32c3 100644 --- a/src/upnp/upnp_context.cpp +++ b/src/upnp/upnp_context.cpp @@ -262,10 +262,11 @@ UPnPContext::setPublicAddress(const IpAddr& addr) if (not addr) return; - JAMI_DBG("Setting the known public address to %s", addr.toString().c_str()); - std::lock_guard<std::mutex> lock(mappingMutex_); - knownPublicAddress_ = std::move(addr); + if (knownPublicAddress_ != addr) { + knownPublicAddress_ = std::move(addr); + JAMI_DBG("Setting the known public address to %s", addr.toString().c_str()); + } } bool