conference: properly protect confInfo_

Change-Id: Ifb897e5918163fe9d6ee7eefb9e65a76c441d8dc

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_);
         }

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());
             }
-            }
 
-            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_);
+    std::lock_guard<std::mutex> lk(confInfoMutex_);
     for (auto& info : confInfo_) {
-            auto uri = info.uri;
-            uri = string_remove_suffix(uri, '@');
-            info.isModerator = isModerator(uri);
-        }
+        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,15 +679,13 @@ Conference::muteParticipant(const std::string& uri, const bool& state, const std
 void
 Conference::updateMuted()
 {
-    {
-        std::lock_guard<std::mutex> lk2(confInfoMutex_);
+    std::lock_guard<std::mutex> lk(confInfoMutex_);
     for (auto& info : confInfo_) {
         auto uri = string_remove_suffix(info.uri, '@');
         if (uri.empty())
-                uri = "host";
+            uri = "host"sv;
         info.audioMuted = isMuted(uri);
     }
-    }
     sendConferenceInfos();
 }
 
@@ -735,11 +713,8 @@ 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) {
+            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;

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_;