From bd04887eaa6840b051273132885af6679f02fc1c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Fri, 6 Mar 2020 11:38:45 -0500
Subject: [PATCH] jamiaccount: avoid to store two SIP connections with same
channel
This fix a recurrent segfault where the ICE channel was null (should never
be the case)
Change-Id: I6420989b300d7ce9769bdf971613daa11afb6f78
Gitlab: #205
---
src/jamidht/jamiaccount.cpp | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp
index df75d68763..58ab39ecfd 100644
--- a/src/jamidht/jamiaccount.cpp
+++ b/src/jamidht/jamiaccount.cpp
@@ -484,6 +484,10 @@ JamiAccount::startOutgoingCall(const std::shared_ptr<SIPCall>& call, const std::
auto& it = deviceConnIt->second.back();
auto transport = it.transport;
+ if (!it.channel->underlyingICE()) {
+ JAMI_WARN("A SIP transport exists without Channel, this is a bug. Please report");
+ continue;
+ }
if (!transport) continue;
call->setTransport(transport);
@@ -3181,10 +3185,12 @@ JamiAccount::requestSIPConnection(const std::string& peerId, const std::string&
JAMI_INFO("Ask %s for a new SIP channel", deviceId.c_str());
if (!connectionManager_) return;
connectionManager_->connectDevice(deviceId, "sip",
- [w=weak(), id](std::shared_ptr<ChannelSocket> socket) {
+ [w=weak(), id](std::shared_ptr<ChannelSocket>) {
auto shared = w.lock();
if (!shared) return;
- if (socket) shared->cacheSIPConnection(std::move(socket), id.first, id.second);
+ // NOTE: No need to cache Connection there. OnConnectionReady
+ // is called before this callback, so the socket is already
+ // cached if succeed. We just need to remove the pending request.
std::lock_guard<std::mutex> lk(shared->sipConnectionsMtx_);
shared->pendingSipConnections_.erase(id);
});
@@ -3194,6 +3200,16 @@ void
JamiAccount::cacheSIPConnection(std::shared_ptr<ChannelSocket>&& socket, const std::string& peerId, const std::string& deviceId)
{
std::unique_lock<std::mutex> lk(sipConnectionsMtx_);
+ // Verify that the connection is not already cached
+ auto& connections = sipConnections_[peerId][deviceId];
+ auto conn = std::find_if(connections.begin(), connections.end(), [socket](auto v) {
+ return v.channel == socket;
+ });
+ if (conn != connections.end()) {
+ JAMI_WARN("Channel socket already cached with this peer");
+ return;
+ }
+
// Convert to SIP transport
sip_utils::register_thread();
auto onShutdown = [w=weak(), peerId, deviceId, socket]() {
@@ -3201,11 +3217,12 @@ JamiAccount::cacheSIPConnection(std::shared_ptr<ChannelSocket>&& socket, const s
if (!shared) return;
std::lock_guard<std::mutex> lk(shared->sipConnectionsMtx_);
auto& connections = shared->sipConnections_[peerId][deviceId];
- auto conn = std::find_if(connections.begin(), connections.end(), [socket](auto v) {
- return v.channel == socket;
- });
- if (conn != connections.end()) {
- connections.erase(conn);
+ auto conn = connections.begin();
+ while (conn != connections.end()) {
+ if (conn->channel == socket)
+ conn = connections.erase(conn);
+ else
+ conn++;
}
};
auto sip_tr = link_->sipTransportBroker->getChanneledTransport(socket, std::move(onShutdown));
--
GitLab