From 36925c0d94c7cd14819df8883cae444b09550060 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Wed, 28 Feb 2024 15:13:00 -0500
Subject: [PATCH] conversation: avoid to get lastCommitId while merging
Change-Id: I2a7b7cc13268be127a6afe3d689d005b6596a3cd
---
src/jamidht/conversation.cpp | 4 ++--
src/jamidht/conversation_module.cpp | 6 +++---
.../conversation/conversationMembersEvent.cpp | 11 +++++++----
3 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/src/jamidht/conversation.cpp b/src/jamidht/conversation.cpp
index 86f5d3ae63..2a027614b3 100644
--- a/src/jamidht/conversation.cpp
+++ b/src/jamidht/conversation.cpp
@@ -1596,10 +1596,10 @@ Conversation::lastCommitId() const
options.nbOfCommits = 1;
options.skipMerge = true;
History optHistory;
+ std::lock_guard wlk(pimpl_->writeMtx_); // Avoid any write during a loading operation
std::lock_guard lk(pimpl_->historyMtx_);
- if (!pimpl_->loadedHistory_.messageList.empty()) {
+ if (!pimpl_->loadedHistory_.messageList.empty())
return (*pimpl_->loadedHistory_.messageList.begin())->id;
- }
auto res = pimpl_->loadMessages2(options, &optHistory);
if (res.empty())
return {};
diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp
index 12b556ec02..ad75b45059 100644
--- a/src/jamidht/conversation_module.cpp
+++ b/src/jamidht/conversation_module.cpp
@@ -872,9 +872,9 @@ ConversationModule::Impl::removeRepositoryImpl(SyncedConversation& conv, bool sy
JAMI_LOG("Remove conversation: {}", conv.info.id);
try {
if (conv.conversation->mode() == ConversationMode::ONE_TO_ONE) {
- auto account = account_.lock();
for (const auto& member : conv.conversation->getInitialMembers()) {
- if (member != account->getUsername()) {
+ auto account = account_.lock();
+ if (member != account->getUsername()) {
// Note: this can happen while re-adding a contact.
// In this case, check that we are removing the linked conversation.
if (conv.info.id == getOneToOneConversation(member)) {
@@ -2254,7 +2254,7 @@ ConversationModule::onSyncData(const SyncMsg& msg,
// If multi devices, it can detect a conversation that was already
// removed, so just check if the convinfo contains a removed conv
if (conv->info.removed) {
- if (conv->info.removed > convInfo.created) {
+ if (conv->info.removed >= convInfo.created) {
// Only reclone if re-added, else the peer is not synced yet (could be
// offline before)
continue;
diff --git a/test/unitTest/conversation/conversationMembersEvent.cpp b/test/unitTest/conversation/conversationMembersEvent.cpp
index e85aaf0aa9..f3737e377d 100644
--- a/test/unitTest/conversation/conversationMembersEvent.cpp
+++ b/test/unitTest/conversation/conversationMembersEvent.cpp
@@ -576,6 +576,8 @@ ConversationMembersEventTest::testAddOfflineMemberThenConnects()
auto aliceMsgSize = aliceData.messages.size();
libjami::addConversationMember(aliceId, convId, carlaUri);
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return aliceMsgSize + 1 == aliceData.messages.size(); }));
+ CPPUNIT_ASSERT(!cv.wait_for(lk, 30s, [&] { return carlaData.requestReceived; }));
+
Manager::instance().sendRegister(carlaId, true);
CPPUNIT_ASSERT(cv.wait_for(lk, 60s, [&] { return carlaData.requestReceived; }));
@@ -929,7 +931,8 @@ ConversationMembersEventTest::testMemberCannotBanOther()
// remove from member & add into banned without voting for the ban
simulateRemoval(carlaAccount, convId, bobUri);
// Note: it may be possible that alice doesn't get the error if they got messages from bob (and bob rejects due to an error)
- CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return aliceData.errorDetected || bobData.errorDetected; }));
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() {
+ return aliceData.errorDetected || bobData.errorDetected; }));
auto bobMsgSize = bobData.messages.size();
libjami::sendMessage(aliceId, convId, "hi"s, "");
@@ -966,7 +969,7 @@ ConversationMembersEventTest::testMemberCannotUnBanOther()
cv.wait_for(lk, 30s, [&]() { return !carlaData.conversationId.empty()
&& aliceData.members[carlaUri] == 1
&& bobData.members[carlaUri] == 1; }));
-
+ std::this_thread::sleep_for(3s); // Wait that carla finish the clone
// Now check that alice, has the only admin, can remove bob
libjami::removeConversationMember(aliceId, convId, bobUri);
CPPUNIT_ASSERT(
@@ -1443,7 +1446,8 @@ ConversationMembersEventTest::testAvoidTwoOneToOneMultiDevices()
bobAccount->addContact(aliceUri);
bobAccount->sendTrustRequest(aliceUri, {});
CPPUNIT_ASSERT(
- cv.wait_for(lk, 30s, [&]() { return bobData.conversationId == aliceData.conversationId && bob2Data.conversationId == aliceData.conversationId; }));
+ cv.wait_for(lk, 30s, [&]() {
+ return bobData.conversationId == aliceData.conversationId && bob2Data.conversationId == aliceData.conversationId; }));
}
void
@@ -1696,7 +1700,6 @@ ConversationMembersEventTest::testBanFromNewDevice()
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bob2Data.deviceAnnounced; }));
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return !bob2Data.conversationId.empty(); }));
-
auto bobMsgSize = bobData.messages.size();
libjami::removeConversationMember(bob2Id, convId, aliceUri);
CPPUNIT_ASSERT(
--
GitLab