From 1fa0fd9f33e697bf3f8b89b4b9042528c516255e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
 <sebastien.blin@savoirfairelinux.com>
Date: Thu, 4 Jan 2024 08:47:26 -0500
Subject: [PATCH] conversation_module: fix double unlock crash

if (acc && isOneToOne) the lock was unlocked before
addConversationRequest

Change-Id: I670dadf806ff2c949d8260c83cd9cce42ab1b3d6
---
 src/jamidht/conversation_module.cpp | 41 +++++++++++++++--------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp
index 2c103fd57f..75a2863547 100644
--- a/src/jamidht/conversation_module.cpp
+++ b/src/jamidht/conversation_module.cpp
@@ -1708,14 +1708,19 @@ ConversationModule::onTrustRequest(const std::string& uri,
 void
 ConversationModule::onConversationRequest(const std::string& from, const Json::Value& value)
 {
-    std::unique_lock<std::mutex> lk(pimpl_->conversationsRequestsMtx_);
     ConversationRequest req(value);
+    auto acc = pimpl_->account_.lock();
+    auto isOneToOne = req.isOneToOne();
+    std::string oldConv;
+    if (acc && isOneToOne) {
+        oldConv = getOneToOneConversation(from);
+    }
+    std::unique_lock<std::mutex> lk(pimpl_->conversationsRequestsMtx_);
     JAMI_DEBUG("[Account {}] Receive a new conversation request for conversation {} from {}",
               pimpl_->accountId_,
               req.conversationId,
               from);
     auto convId = req.conversationId;
-    req.from = from;
 
     // Already accepted request, do nothing
     if (pimpl_->isConversation(convId))
@@ -1727,29 +1732,25 @@ ConversationModule::onConversationRequest(const std::string& from, const Json::V
                   pimpl_->accountId_, static_cast<int>(oldReq->declined));
         return;
     }
-    req.received = std::time(nullptr);
-    auto reqMap = req.toMap();
-    auto isOneToOne = req.isOneToOne();
-    std::string oldConv;
-    auto acc = pimpl_->account_.lock();
-    if (acc && isOneToOne) {
+
+    if (!oldConv.empty()) {
         lk.unlock();
-        auto oldConv = getOneToOneConversation(from);
-        if (!oldConv.empty()) {
-            // Already a conversation with the contact.
-            if (oldConv != convId && acc->updateConvForContact(from, oldConv, convId)) {
-                initReplay(oldConv, convId);
-                cloneConversationFrom(convId, from, oldConv);
-                return;
-            }
-            // If there is already an active one to one conversation here, it's an active
-            // contact and the contact will reclone this activeConv, so ignore the request
-            JAMI_WARNING("Contact is sending a request for a non active conversation. Ignore. They will "
-                    "clone the old one");
+        // Already a conversation with the contact.
+        if (oldConv != convId && acc->updateConvForContact(from, oldConv, convId)) {
+            initReplay(oldConv, convId);
+            cloneConversationFrom(convId, from, oldConv);
             return;
         }
+        // If there is already an active one to one conversation here, it's an active
+        // contact and the contact will reclone this activeConv, so ignore the request
+        JAMI_WARNING("Contact is sending a request for a non active conversation. Ignore. They will "
+                "clone the old one");
+        return;
     }
 
+    req.received = std::time(nullptr);
+    req.from = from;
+    auto reqMap = req.toMap();
     if (pimpl_->addConversationRequest(convId, std::move(req))) {
         lk.unlock();
         // Note: no need to sync here because other connected devices should receive
-- 
GitLab