From b34cd486e27430a5535d4dc48eb4e6297626d92d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adrien=20B=C3=A9raud?= <adrien.beraud@savoirfairelinux.com>
Date: Tue, 22 Dec 2020 16:38:37 -0500
Subject: [PATCH] conference: properly protect confInfo_
Change-Id: Ifb897e5918163fe9d6ee7eefb9e65a76c441d8dc
---
src/call.cpp | 12 ++----
src/conference.cpp | 99 +++++++++++++++++-----------------------------
src/conference.h | 3 +-
3 files changed, 42 insertions(+), 72 deletions(-)
diff --git a/src/call.cpp b/src/call.cpp
index 38687832e5..907bea10e1 100644
--- a/src/call.cpp
+++ b/src/call.cpp
@@ -648,17 +648,13 @@ Call::setConferenceInfo(const std::string& msg)
if (confID_.empty()) {
// confID_ empty -> participant set confInfo with the received one
confInfo_ = std::move(newInfo);
- std::vector<std::map<std::string, std::string>> toSend =
- confInfo_.toVectorMapStringString();
-
// Inform client that layout has changed
- jami::emitSignal<DRing::CallSignal::OnConferenceInfosUpdated>(id_, std::move(toSend));
+ jami::emitSignal<DRing::CallSignal::OnConferenceInfosUpdated>(id_, confInfo_.toVectorMapStringString());
} else {
// confID_ not empty -> host merge confInfo with the received confInfo
- auto oldInfo = confInfo_;
for (auto& newI : newInfo) {
bool isNewParticipant = true;
- for (auto& oldI : oldInfo) {
+ for (auto& oldI : confInfo_) {
if (newI.uri == oldI.uri) {
oldI = newI;
isNewParticipant = false;
@@ -669,11 +665,9 @@ Call::setConferenceInfo(const std::string& msg)
// ParticipantInfo not present in confInfo -> the sender of newInfo ...
// is currently hosting another conference. Add the unknown participant ...
// to the confInfo
- oldInfo.emplace_back(newI);
+ confInfo_.emplace_back(newI);
}
}
- confInfo_ = std::move(oldInfo);
-
if (auto conf = Manager::instance().getConferenceFromID(confID_))
conf->updateConferenceInfo(confInfo_);
}
diff --git a/src/conference.cpp b/src/conference.cpp
index e860e519b9..90b17b3dc0 100644
--- a/src/conference.cpp
+++ b/src/conference.cpp
@@ -92,11 +92,10 @@ Conference::Conference()
if (auto videoMixer = shared->getVideoMixer())
active = info.source == videoMixer->getActiveParticipant();
subCalls.erase(it->second);
- std::string_view partURI = uri;
- partURI = string_remove_suffix(partURI, '@');
+ std::string_view partURI = string_remove_suffix(uri, '@');
auto isModerator = shared->isModerator(partURI);
if (uri.empty())
- partURI = "host";
+ partURI = "host"sv;
auto isMuted = shared->isMuted(partURI);
newInfo.emplace_back(ParticipantInfo {std::move(uri),
"",
@@ -120,12 +119,7 @@ Conference::Conference()
ParticipantInfo {std::move(uri), "", false, 0, 0, 0, 0, true, false, isModerator});
}
- {
- std::lock_guard<std::mutex> lk2(shared->confInfoMutex_);
- shared->confInfo_ = std::move(newInfo);
- }
-
- shared->sendConferenceInfos();
+ shared->updateConferenceInfo(std::move(newInfo));
});
});
#endif
@@ -184,9 +178,7 @@ Conference::add(const std::string& participant_id)
// Check if participant was muted before conference
if (auto call = Manager::instance().callFactory.getCall<SIPCall>(participant_id)) {
if (call->isPeerMuted()) {
- auto uri = call->getPeerNumber();
- uri = string_remove_suffix(uri, '@');
- participantsMuted_.emplace(uri);
+ participantsMuted_.emplace(string_remove_suffix(call->getPeerNumber(), '@'));
}
}
#ifdef ENABLE_VIDEO
@@ -256,11 +248,8 @@ ConfInfo::toVectorMapStringString() const
{
std::vector<std::map<std::string, std::string>> infos;
infos.reserve(size());
- auto it = cbegin();
- while (it != cend()) {
- infos.emplace_back(it->toMap());
- ++it;
- }
+ for (const auto& info : *this)
+ infos.emplace_back(info.toMap());
return infos;
}
@@ -277,20 +266,19 @@ Conference::sendConferenceInfos()
if (!account)
continue;
- ConfInfo confInfo = std::move(getConfInfoHostUri(account->getUsername()+ "@ring.dht"));
+ ConfInfo confInfo = getConfInfoHostUri(account->getUsername()+ "@ring.dht");
Json::Value jsonArray = {};
- {
- std::lock_guard<std::mutex> lk2(confInfoMutex_);
- for (const auto& info : confInfo) {
- jsonArray.append(info.toJson());
- }
+ for (const auto& info : confInfo) {
+ jsonArray.append(info.toJson());
}
- Json::StreamWriterBuilder builder = {};
- const auto confInfoStr = Json::writeString(builder, jsonArray);
- call->sendTextMessage(std::map<std::string, std::string> {{"application/confInfo+json",
- confInfoStr}},
- account->getFromUri());
+ runOnMainThread([
+ call,
+ confInfoStr = Json::writeString(Json::StreamWriterBuilder{}, jsonArray),
+ from = account->getFromUri()
+ ] {
+ call->sendTextMessage({{"application/confInfo+json", confInfoStr}}, from);
+ });
}
}
@@ -417,8 +405,7 @@ Conference::bindHost()
for (const auto& item : participants_) {
if (auto call = Manager::instance().getCallFromCallID(item)) {
- std::string_view uri = call->getPeerNumber();
- uri = string_remove_suffix(uri, '@');
+ auto uri = string_remove_suffix(call->getPeerNumber(), '@');
if (isMuted(uri))
continue;
rbPool.bindCallID(item, RingBufferPool::DEFAULT_ID);
@@ -547,8 +534,7 @@ Conference::onConfOrder(const std::string& callId, const std::string& confOrder)
{
// Check if the peer is a master
if (auto call = Manager::instance().getCallFromCallID(callId)) {
- std::string_view uri = call->getPeerNumber();
- uri = string_remove_suffix(uri, '@');
+ auto uri = string_remove_suffix(call->getPeerNumber(), '@');
if (!isModerator(uri)) {
JAMI_WARN("Received conference order from a non master (%s)", uri.data());
return;
@@ -593,8 +579,7 @@ Conference::setModerator(const std::string& uri, const bool& state)
{
for (const auto& p : participants_) {
if (auto call = Manager::instance().callFactory.getCall<SIPCall>(p)) {
- std::string_view partURI = call->getPeerNumber();
- partURI = string_remove_suffix(partURI, '@');
+ auto partURI = string_remove_suffix(call->getPeerNumber(), '@');
if (partURI == uri) {
if (state and not isModerator(uri)) {
JAMI_DBG("Add %s as moderator", partURI.data());
@@ -615,13 +600,9 @@ Conference::setModerator(const std::string& uri, const bool& state)
void
Conference::updateModerators()
{
- {
- std::lock_guard<std::mutex> lk2(confInfoMutex_);
- for (auto& info : confInfo_) {
- auto uri = info.uri;
- uri = string_remove_suffix(uri, '@');
- info.isModerator = isModerator(uri);
- }
+ std::lock_guard<std::mutex> lk(confInfoMutex_);
+ for (auto& info : confInfo_) {
+ info.isModerator = isModerator(string_remove_suffix(info.uri, '@'));
}
sendConferenceInfos();
}
@@ -673,19 +654,18 @@ Conference::muteParticipant(const std::string& uri, const bool& state, const std
}
// Mute participant
+ auto peerURI = string_remove_suffix(uri, '@');
for (const auto& p : participants_) {
- std::string_view peerURI = string_remove_suffix(uri, '@');
if (auto call = Manager::instance().callFactory.getCall<SIPCall>(p)) {
- std::string_view partURI = call->getPeerNumber();
- partURI = string_remove_suffix(partURI, '@');
+ auto partURI = string_remove_suffix(call->getPeerNumber(), '@');
if (partURI == peerURI) {
if (state and not isMuted(partURI)) {
- JAMI_DBG("Mute participant %s", partURI.data());
+ JAMI_DBG("Mute participant %.*s", (int)partURI.size(), partURI.data());
participantsMuted_.emplace(std::string(partURI));
unbindParticipant(p);
updateMuted();
} else if (not state and isMuted(partURI)) {
- JAMI_DBG("Unmute participant %s", partURI.data());
+ JAMI_DBG("Unmute participant %.*s", (int)partURI.size(), partURI.data());
participantsMuted_.erase(std::string(partURI));
bindParticipant(p);
updateMuted();
@@ -699,14 +679,12 @@ Conference::muteParticipant(const std::string& uri, const bool& state, const std
void
Conference::updateMuted()
{
- {
- std::lock_guard<std::mutex> lk2(confInfoMutex_);
- for (auto& info : confInfo_) {
- auto uri = string_remove_suffix(info.uri, '@');
- if (uri.empty())
- uri = "host";
- info.audioMuted = isMuted(uri);
- }
+ std::lock_guard<std::mutex> lk(confInfoMutex_);
+ for (auto& info : confInfo_) {
+ auto uri = string_remove_suffix(info.uri, '@');
+ if (uri.empty())
+ uri = "host"sv;
+ info.audioMuted = isMuted(uri);
}
sendConferenceInfos();
}
@@ -735,12 +713,9 @@ Conference::isHost(std::string_view uri) const
// (a local URI can be in the call with another device)
for (const auto& p : participants_) {
if (auto call = Manager::instance().callFactory.getCall<SIPCall>(p)) {
- auto w = call->getAccount();
- auto account = w.lock();
- if (!account)
- continue;
- if (account->getUsername() == uri) {
- return true;
+ if (auto account = call->getAccount().lock()) {
+ if (account->getUsername() == uri)
+ return true;
}
}
}
@@ -750,6 +725,7 @@ Conference::isHost(std::string_view uri) const
void
Conference::updateConferenceInfo(ConfInfo confInfo)
{
+ std::lock_guard<std::mutex> lk(confInfoMutex_);
confInfo_ = std::move(confInfo);
sendConferenceInfos();
}
@@ -764,8 +740,7 @@ Conference::hangupParticipant(const std::string& participant_id)
for (const auto& p : participants_) {
if (auto call = Manager::instance().callFactory.getCall<SIPCall>(p)) {
- std::string_view partURI = call->getPeerNumber();
- partURI = string_remove_suffix(partURI, '@');
+ std::string_view partURI = string_remove_suffix(call->getPeerNumber(), '@');
if (partURI == participant_id) {
Manager::instance().hangupCall(call->getCallId());
return;
diff --git a/src/conference.h b/src/conference.h
index cb7801a726..e50ef0c5d0 100644
--- a/src/conference.h
+++ b/src/conference.h
@@ -254,7 +254,8 @@ private:
ParticipantSet participants_;
mutable std::mutex confInfoMutex_ {};
- mutable ConfInfo confInfo_ {};
+ ConfInfo confInfo_ {};
+
void sendConferenceInfos();
// We need to convert call to frame
std::mutex videoToCallMtx_;
--
GitLab