From 5482ea5754e4916537c71f44878f7aebbe7b58f6 Mon Sep 17 00:00:00 2001
From: Olivier Dion <olivier.dion@savoirfairelinux.com>
Date: Mon, 19 Jul 2021 10:29:30 -0400
Subject: [PATCH] sipcall: Refactor deletion of ICE transports
Change-Id: Icb4266aeb1274c02d8fc7e60dabfc7a7bec7a17e
---
src/sip/sipcall.cpp | 50 ++++++++++++++++++++++++++++-----------------
src/sip/sipcall.h | 2 ++
2 files changed, 33 insertions(+), 19 deletions(-)
diff --git a/src/sip/sipcall.cpp b/src/sip/sipcall.cpp
index fb8cad6483..6175ef252d 100644
--- a/src/sip/sipcall.cpp
+++ b/src/sip/sipcall.cpp
@@ -174,9 +174,7 @@ SIPCall::~SIPCall()
std::lock_guard<std::recursive_mutex> lk {callMutex_};
{
std::lock_guard<std::mutex> lk(transportMtx_);
- if (tmpMediaTransport_)
- dht::ThreadPool::io().run([ice = std::make_shared<decltype(tmpMediaTransport_)>(
- std::move(tmpMediaTransport_))] {});
+ resetTransport(std::move(tmpMediaTransport_));
}
setTransport({});
setInviteSession(); // prevents callback usage
@@ -1450,11 +1448,13 @@ SIPCall::removeCall()
sdp_->setActiveRemoteSdpSession(nullptr);
}
Call::removeCall();
- if (mediaTransport_)
- dht::ThreadPool::io().run([ice = std::move(mediaTransport_)] {});
- if (tmpMediaTransport_)
- dht::ThreadPool::io().run([ice = std::make_shared<decltype(tmpMediaTransport_)>(
- std::move(tmpMediaTransport_))] {});
+
+ {
+ std::lock_guard<std::mutex> lk(transportMtx_);
+ resetTransport(std::move(mediaTransport_));
+ resetTransport(std::move(tmpMediaTransport_));
+ }
+
setInviteSession();
setTransport({});
}
@@ -2350,12 +2350,9 @@ SIPCall::onIceNegoSucceed()
// Nego succeed: move to the new media transport
stopAllMedia();
{
- std::lock_guard<std::mutex> lk(transportMtx_);
+ std::unique_lock<std::mutex> lk(transportMtx_);
if (tmpMediaTransport_) {
- // Destroy the ICE media transport on another thread. This can take
- // quite some time.
- if (mediaTransport_)
- dht::ThreadPool::io().run([ice = std::move(mediaTransport_)] {});
+ resetTransport(std::move(mediaTransport_));
mediaTransport_ = std::move(tmpMediaTransport_);
}
}
@@ -2892,21 +2889,36 @@ SIPCall::initIceMediaTransport(bool master, std::optional<IceTransportOptions> o
// Each RTP stream requires a pair of ICE components (RTP + RTCP).
iceOptions.compCountPerStream = ICE_COMP_COUNT_PER_STREAM;
auto& iceTransportFactory = Manager::instance().getIceTransportFactory();
- std::lock_guard<std::mutex> lk(transportMtx_);
+
auto transport = iceTransportFactory.createUTransport(getCallId().c_str(), iceOptions);
+
// Destroy old ice on a separate io pool
- if (tmpMediaTransport_)
- dht::ThreadPool::io().run([ice = std::make_shared<decltype(tmpMediaTransport_)>(
- std::move(tmpMediaTransport_))] {});
- tmpMediaTransport_ = std::move(transport);
+ {
+ std::lock_guard<std::mutex> lk(transportMtx_);
+ resetTransport(std::move(tmpMediaTransport_));
+ tmpMediaTransport_ = std::move(transport);
+ }
+
if (tmpMediaTransport_) {
- JAMI_DBG("[call:%s] Successfully created media ICE transport", getCallId().c_str());
+ JAMI_DBG("[call:%s] Successfully created media ICE transport [ice:%p]", getCallId().c_str(), tmpMediaTransport_.get());
} else {
JAMI_ERR("[call:%s] Failed to create media ICE transport", getCallId().c_str());
}
+
return static_cast<bool>(tmpMediaTransport_);
}
+void
+SIPCall::resetTransport(std::shared_ptr<IceTransport>&& transport)
+{
+ // Move the transport to another thread and destroy it there if possible
+ if (transport) {
+ dht::ThreadPool::io().run([transport = std::move(transport)]() mutable {
+ transport.reset();
+ });
+ }
+}
+
void
SIPCall::merge(Call& call)
{
diff --git a/src/sip/sipcall.h b/src/sip/sipcall.h
index c5b0954d7c..80b60d91e3 100644
--- a/src/sip/sipcall.h
+++ b/src/sip/sipcall.h
@@ -124,6 +124,8 @@ public:
private:
void merge(Call& call) override; // not public - only called by Call
+ void resetTransport(std::shared_ptr<IceTransport>&& transport);
+
public:
void answer() override;
void answer(const std::vector<DRing::MediaMap>& mediaList) override;
--
GitLab