conversationmodel: fix deadlock on first interaction

af04c34f introduced a condition variable used to block until the
conversation was created, circumventing a race condition when
attempting to insert interactions. However, when not using D-Bus,
slotContactAdded will be invoked on the blocked thread, and thus
deadlock until the wait_for timeout.

This patch replaces the synchronization mechanism with callbacks.

Change-Id: I5f975a6245232e586be7df3f29008c1162d6b208
Reviewed-by: Sebastien Blin <sebastien.blin@savoirfairelinux.com>

src/api/conversationmodel.h

@@ -249,6 +249,12 @@ Q_SIGNALS:
      * @note the client must connect this signal to know when update the view of the list
      */
     void allHistoryCleared() const;
+    /**
+     * Emitted at the end of slotContactAdded to notify that an existing conversation can
+     * be modified
+     * @param uid
+     */
+    void conversationReady(std::string uid) const;
 
 private:
     std::unique_ptr<ConversationModelPimpl> pimpl_;

src/conversationmodel.cpp

@@ -177,10 +177,6 @@ public:
     std::pair<bool, bool> dirtyConversations {true, true}; ///< true if filteredConversations/customFilteredConversations must be regenerated
     std::map<std::string, std::mutex> interactionsLocks; ///< {convId, mutex}
 
-    // Synchronization tools for account creation synchronization
-    std::mutex m_mutex_conversations;
-    std::condition_variable m_condVar_conversation_creation;
-
 public Q_SLOTS:
     /**
      * Listen from contactModel when updated (like new alias, avatar, etc.)
@@ -571,22 +567,8 @@ ConversationModelPimpl::placeCall(const std::string& uid, bool isAudioOnly)
         url = "ring:" + url; // Add the ring: before or it will fail.
     }
 
-    if (isTemporary) {
-        /* Block until we are sure that the final conversation was created by
-           slotContactAdded(). If adding contact failed we should not process
-           any further */
-        std::unique_lock<std::mutex> lock(m_mutex_conversations);
-        auto res = m_condVar_conversation_creation.wait_for(lock, std::chrono::seconds(2), [&]() {
-            return indexOfContact(convId) >= 0;
-        });
-        lock.unlock();
-
-        if (!res) {
-            qDebug() << "ConversationModelPimpl::placeCall reached timeout while waiting for contact to be added. Couldn't place call.";
-            return;
-        }
-    }
-
+    auto cb = std::function<void(std::string)>(
+        [this, isTemporary, url, isAudioOnly, &conversation](std::string convId) {
             int contactIndex;
             if (isTemporary && (contactIndex = indexOfContact(convId)) < 0) {
                 qDebug() << "Can't place call: Other participant is not a contact (removed while placing call ?)";
@@ -604,6 +586,21 @@ ConversationModelPimpl::placeCall(const std::string& uid, bool isAudioOnly)
 
             dirtyConversations = { true, true };
             emit behaviorController.showIncomingCallView(linked.owner.id, newConv);
+        });
+
+    if (isTemporary) {
+        QMetaObject::Connection* const connection = new QMetaObject::Connection;
+        *connection = connect(&this->linked, &ConversationModel::conversationReady,
+            [cb, connection](std::string convId) {
+                cb(convId);
+                QObject::disconnect(*connection);
+                if (connection) {
+                    delete connection;
+                }
+            });
+    } else {
+        cb(convId);
+    }
 }
 
 void
@@ -638,9 +635,6 @@ ConversationModel::sendMessage(const std::string& uid, const std::string& body)
     auto accountId = pimpl_->accountProfileId;
     bool isTemporary = conversation.participants.front() == "";
 
-    uint64_t daemonMsgId = 0;
-    auto status = interaction::Status::SENDING;
-
     /* Make a copy of participants list: if current conversation is temporary,
        it might me destroyed while we are reading it */
     const auto participants = conversation.participants;
@@ -658,22 +652,8 @@ ConversationModel::sendMessage(const std::string& uid, const std::string& body)
         pimpl_->sendContactRequest(participant);
     }
 
-    if (isTemporary) {
-        /* Block until we are sure that the final conversation was created by
-           slotContactAdded(). If adding contact failed we should not process
-           any further */
-        std::unique_lock<std::mutex> lock(pimpl_->m_mutex_conversations);
-        auto res = pimpl_->m_condVar_conversation_creation.wait_for(lock, std::chrono::seconds(2), [&]() {
-            return pimpl_->indexOfContact(convId) >= 0;
-        });
-        lock.unlock();
-
-        if (!res) {
-            qDebug() << "ConversationModel::sendMessage reached timeout while waiting for contact to be added. Couldn't send message.";
-            return;
-        }
-    }
-
+    auto cb = std::function<void(std::string)>(
+        [this, accountId, isTemporary, body, &conversation](std::string convId) {
             /* Now we should be able to retrieve the final conversation, in case the previous
                one was temporary */
                // FIXME potential race condition between index check and at() call
@@ -683,6 +663,9 @@ ConversationModel::sendMessage(const std::string& uid, const std::string& body)
                 return;
             }
 
+            uint64_t daemonMsgId = 0;
+            auto status = interaction::Status::SENDING;
+
             auto& newConv = isTemporary ? pimpl_->conversations.at(contactIndex) : conversation;
             convId = newConv.uid;
 
@@ -738,7 +721,21 @@ ConversationModel::sendMessage(const std::string& uid, const std::string& body)
             pimpl_->sortConversations();
             // The order has changed, informs the client to redraw the list
             emit modelSorted();
+        });
 
+    if (isTemporary) {
+        QMetaObject::Connection* const connection = new QMetaObject::Connection;
+        *connection = connect(this, &ConversationModel::conversationReady,
+            [cb, connection](std::string convId) {
+                cb(convId);
+                QObject::disconnect(*connection);
+                if (connection) {
+                    delete connection;
+                }
+            });
+    } else {
+        cb(convId);
+    }
 }
 
 void
@@ -1288,7 +1285,7 @@ ConversationModelPimpl::slotContactAdded(const std::string& uri)
     }
 
     sortConversations();
-    m_condVar_conversation_creation.notify_all();
+    emit linked.conversationReady(uri);
     emit linked.modelSorted();
 }
 
@@ -1761,23 +1758,10 @@ ConversationModel::sendFile(const std::string& convUid,
 
     pimpl_->sendContactRequest(peerUri);
 
-    if (isTemporary) {
-        /* Block until we are sure that the final conversation was created by
-           slotContactAdded(). If adding contact failed we should not process
-           any further */
-        std::unique_lock<std::mutex> lock(pimpl_->m_mutex_conversations);
-        auto res = pimpl_->m_condVar_conversation_creation.wait_for(lock, std::chrono::seconds(2),
-                   [&]() { return pimpl_->indexOfContact(convUidCopy) >= 0; });
-        lock.unlock();
-
-        if (!res) {
-            qDebug() << "ConversationModel::sendFile reached timeout while waiting for contact to be added. Couldn't send file.";
-            return;
-        }
-    }
-
+    auto cb = std::function<void(std::string)>(
+        [this, isTemporary, peerUri, path, filename](std::string convId) {
             int contactIndex;
-    if (isTemporary && (contactIndex = pimpl_->indexOfContact(convUidCopy)) < 0) {
+            if (isTemporary && (contactIndex = pimpl_->indexOfContact(convId)) < 0) {
                 qDebug() << "Can't send file: Other participant is not a contact (removed while sending file ?)";
                 return;
             }
@@ -1795,6 +1779,21 @@ ConversationModel::sendFile(const std::string& convUid,
                                                         newPeerUri.c_str(),
                                                         path.c_str(),
                                                         filename.c_str());
+        });
+
+    if (isTemporary) {
+        QMetaObject::Connection* const connection = new QMetaObject::Connection;
+        *connection = connect(this, &ConversationModel::conversationReady,
+            [cb, connection](std::string convId) {
+                cb(convId);
+                QObject::disconnect(*connection);
+                if (connection) {
+                    delete connection;
+                }
+            });
+    } else {
+        cb(convUidCopy);
+    }
 }
 
 void