From dea87f5756797d630538a079db78b3a849449d2c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Thu, 28 Dec 2023 16:06:06 -0500
Subject: [PATCH] gitserver: shutdown socket if data are not sent correctly
Else, socket can hang for nothing.
Moreover, erasePending is moved for the close to be removed as
soon as possible.
This fix sporadic failures in ut_syncHistory
testSyncCreateAccountExportDeleteReimportOldBackup
Change-Id: If230a780d3db93c6b37269a254e339c873fabe92
---
src/jamidht/conversation_module.cpp | 3 ++-
src/jamidht/gitserver.cpp | 39 ++++++++++++++++++++---------
2 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp
index e2712371cf..279a2e6580 100644
--- a/src/jamidht/conversation_module.cpp
+++ b/src/jamidht/conversation_module.cpp
@@ -623,6 +623,7 @@ ConversationModule::Impl::fetchNewCommits(const std::string& peer,
}
{
+
std::lock_guard<std::mutex> lk(conv->mtx);
conv->pending.reset();
// Notify peers that a new commit is there (DRT)
@@ -744,7 +745,7 @@ ConversationModule::Impl::handlePendingConversation(const std::string& conversat
}
if (!commitId.empty())
sendMessageNotification(*conversation, false, commitId);
- lk.unlock();
+ erasePending(); // Will unlock
#ifdef LIBJAMI_TESTABLE
conversation->onBootstrapStatus(bootstrapCbTest_);
diff --git a/src/jamidht/gitserver.cpp b/src/jamidht/gitserver.cpp
index 78f9aa1514..a8762fb58f 100644
--- a/src/jamidht/gitserver.cpp
+++ b/src/jamidht/gitserver.cpp
@@ -51,6 +51,14 @@ public:
, repository_(repository)
, socket_(socket)
{
+ // Check at least if repository is correct
+ git_repository* repo;
+ if (git_repository_open(&repo, repository_.c_str()) != 0) {
+ socket_->shutdown();
+ return;
+ }
+ git_repository_free(repo);
+
socket_->setOnRecv([this](const uint8_t* buf, std::size_t len) {
std::lock_guard<std::mutex> lk(destroyMtx_);
if (isDestroying_)
@@ -204,7 +212,8 @@ GitServer::Impl::sendReferenceCapabilities(bool sendVersion)
// https://github.com/git/git/blob/master/Documentation/technical/pack-protocol.txt#L166
git_repository* repo;
if (git_repository_open(&repo, repository_.c_str()) != 0) {
- JAMI_WARN("Couldn't open %s", repository_.c_str());
+ JAMI_WARNING("Couldn't open {}", repository_);
+ socket_->shutdown();
return;
}
GitRepository rep {repo, git_repository_free};
@@ -221,14 +230,16 @@ GitServer::Impl::sendReferenceCapabilities(bool sendVersion)
packet.str().size(),
ec);
if (ec) {
- JAMI_WARN("Couldn't send data for %s: %s", repository_.c_str(), ec.message().c_str());
+ JAMI_WARNING("Couldn't send data for {}: {}", repository_, ec.message());
+ socket_->shutdown();
return;
}
}
git_oid commit_id;
if (git_reference_name_to_id(&commit_id, rep.get(), "HEAD") < 0) {
- JAMI_ERR("Cannot get reference for HEAD");
+ JAMI_ERROR("Cannot get reference for HEAD");
+ socket_->shutdown();
return;
}
currentHead = git_oid_tostr_s(&commit_id);
@@ -246,7 +257,7 @@ GitServer::Impl::sendReferenceCapabilities(bool sendVersion)
for (std::size_t i = 0; i < refs.count; ++i) {
std::string ref = refs.strings[i];
if (git_reference_name_to_id(&commit_id, rep.get(), ref.c_str()) < 0) {
- JAMI_WARN("Cannot get reference for %s", ref.c_str());
+ JAMI_WARNING("Cannot get reference for {}", ref);
continue;
}
currentHead = git_oid_tostr_s(&commit_id);
@@ -263,7 +274,8 @@ GitServer::Impl::sendReferenceCapabilities(bool sendVersion)
auto toSend = packet.str();
socket_->write(reinterpret_cast<const unsigned char*>(toSend.c_str()), toSend.size(), ec);
if (ec) {
- JAMI_WARN("Couldn't send data for %s: %s", repository_.c_str(), ec.message().c_str());
+ JAMI_WARNING("Couldn't send data for {}: {}", repository_, ec.message());
+ socket_->shutdown();
}
}
@@ -280,7 +292,8 @@ GitServer::Impl::ACKCommon()
auto toSend = packet.str();
socket_->write(reinterpret_cast<const unsigned char*>(toSend.c_str()), toSend.size(), ec);
if (ec) {
- JAMI_WARN("Couldn't send data for %s: %s", repository_.c_str(), ec.message().c_str());
+ JAMI_WARNING("Couldn't send data for {}: {}", repository_, ec.message());
+ socket_->shutdown();
}
}
}
@@ -298,7 +311,8 @@ GitServer::Impl::ACKFirst()
auto toSend = packet.str();
socket_->write(reinterpret_cast<const unsigned char*>(toSend.c_str()), toSend.size(), ec);
if (ec) {
- JAMI_WARN("Couldn't send data for %s: %s", repository_.c_str(), ec.message().c_str());
+ JAMI_WARNING("Couldn't send data for {}: {}", repository_, ec.message());
+ socket_->shutdown();
return false;
}
}
@@ -312,7 +326,8 @@ GitServer::Impl::NAK()
// NAK
socket_->write(reinterpret_cast<const unsigned char*>(NAK_PKT.data()), NAK_PKT.size(), ec);
if (ec) {
- JAMI_WARN("Couldn't send data for %s: %s", repository_.c_str(), ec.message().c_str());
+ JAMI_WARNING("Couldn't send data for {}: {}", repository_, ec.message());
+ socket_->shutdown();
return false;
}
return true;
@@ -330,7 +345,7 @@ GitServer::Impl::sendPackData()
git_packbuilder* pb_ptr;
if (git_packbuilder_new(&pb_ptr, repo.get()) != 0) {
- JAMI_WARN("Couldn't open packbuilder for %s", repository_.c_str());
+ JAMI_WARNING("Couldn't open packbuilder for {}", repository_);
return;
}
GitPackBuilder pb {pb_ptr, git_packbuilder_free};
@@ -338,7 +353,7 @@ GitServer::Impl::sendPackData()
std::string fetched = wantedReference_;
git_oid oid;
if (git_oid_fromstr(&oid, wantedReference_.c_str()) < 0) {
- JAMI_ERR("Cannot get reference for commit %s", wantedReference_.c_str());
+ JAMI_ERROR("Cannot get reference for commit {}", wantedReference_);
return;
}
@@ -409,7 +424,7 @@ GitServer::Impl::sendPackData()
toSendStr.size(),
ec);
if (ec) {
- JAMI_WARN("Couldn't send data for %s: %s", repository_.c_str(), ec.message().c_str());
+ JAMI_WARNING("Couldn't send data for {}: {}", repository_, ec.message());
git_buf_dispose(&data);
return;
}
@@ -420,7 +435,7 @@ GitServer::Impl::sendPackData()
// And finish by a little FLUSH
socket_->write(reinterpret_cast<const uint8_t*>(FLUSH_PKT.data()), FLUSH_PKT.size(), ec);
if (ec) {
- JAMI_WARN("Couldn't send data for %s: %s", repository_.c_str(), ec.message().c_str());
+ JAMI_WARNING("Couldn't send data for {}: {}", repository_, ec.message());
}
// Clear sent data
--
GitLab