From aa4f3abd3212ba5a14b2eea31e0a72eaf0372ea3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Fri, 28 Aug 2020 11:33:17 -0400
Subject: [PATCH] filetransfer: avoid to close files twice
For incoming file, FTP should be the entry point for the daemon to close
the stream. And this info is already stored into ChanneledFileTransfer,
so just protect FtpServer::closeCurrentFile() to avoid to close
the stream twice
Change-Id: I48bc8cc709e40cdff3c5e930ebfacf63c91f20ac
Gitlab: #285
---
src/ftp_server.cpp | 6 ++----
src/ftp_server.h | 2 +-
src/jamidht/channeled_transfers.cpp | 9 ++-------
src/jamidht/channeled_transfers.h | 1 -
src/jamidht/p2p.cpp | 23 ++++-------------------
5 files changed, 9 insertions(+), 32 deletions(-)
diff --git a/src/ftp_server.cpp b/src/ftp_server.cpp
index a44fcba24f..84f2f37af4 100644
--- a/src/ftp_server.cpp
+++ b/src/ftp_server.cpp
@@ -112,10 +112,9 @@ FtpServer::startNewFile()
void
FtpServer::closeCurrentFile()
{
- if (out_.stream) {
+ if (out_.stream && not closed_.exchange(true)) {
out_.stream->close();
out_.stream.reset();
- closed_ = true;
}
}
@@ -123,8 +122,7 @@ bool
FtpServer::read(std::vector<uint8_t>& buffer) const
{
if (!out_.stream) {
- if (closed_) {
- closed_ = false;
+ if (closed_.exchange(false)) {
if (rx_ < fileSize_) {
buffer.resize(4);
buffer[0] = 'N';
diff --git a/src/ftp_server.h b/src/ftp_server.h
index 8269570d6b..137238a4cd 100644
--- a/src/ftp_server.h
+++ b/src/ftp_server.h
@@ -81,7 +81,7 @@ private:
std::stringstream headerStream_;
std::string displayName_;
std::array<char, 1000> line_;
- mutable bool closed_ {false};
+ mutable std::atomic_bool closed_ {false};
mutable bool go_ {false};
FtpState state_ {FtpState::PARSE_HEADERS};
diff --git a/src/jamidht/channeled_transfers.cpp b/src/jamidht/channeled_transfers.cpp
index f0da0a279b..ba18b5c5a9 100644
--- a/src/jamidht/channeled_transfers.cpp
+++ b/src/jamidht/channeled_transfers.cpp
@@ -40,6 +40,7 @@ ChanneledOutgoingTransfer::~ChanneledOutgoingTransfer()
channel_->setOnRecv({});
file_->setOnRecv({});
channel_->shutdown();
+ file_->close();
}
std::string
@@ -99,14 +100,8 @@ ChanneledIncomingTransfer::~ChanneledIncomingTransfer()
{
channel_->setOnRecv({});
channel_->shutdown();
-}
-
-DRing::DataTransferId
-ChanneledIncomingTransfer::id() const
-{
if (ftp_)
- return ftp_->getId();
- return 0;
+ ftp_->close();
}
} // namespace jami
\ No newline at end of file
diff --git a/src/jamidht/channeled_transfers.h b/src/jamidht/channeled_transfers.h
index 1054dc20b6..56c4f7f862 100644
--- a/src/jamidht/channeled_transfers.h
+++ b/src/jamidht/channeled_transfers.h
@@ -53,7 +53,6 @@ public:
const std::shared_ptr<FtpServer>& ftp,
OnStateChangedCb&& cb);
~ChanneledIncomingTransfer();
- DRing::DataTransferId id() const;
private:
std::shared_ptr<FtpServer> ftp_;
diff --git a/src/jamidht/p2p.cpp b/src/jamidht/p2p.cpp
index aca42ce804..30c57e540c 100644
--- a/src/jamidht/p2p.cpp
+++ b/src/jamidht/p2p.cpp
@@ -837,20 +837,14 @@ DhtPeerConnector::Impl::cancelChanneled(const std::string& peerId, const DRing::
if (!shared)
return;
// Cancel outgoing files
- DRing::DataTransferId finalId = tid;
{
std::lock_guard<std::mutex> lk(shared->channeledIncomingMtx_);
- auto it = shared->channeledIncoming_.find(tid);
- if (it != shared->channeledIncoming_.end()) {
- finalId = it->second->id();
- }
- shared->channeledIncoming_.erase(tid);
+ auto it = shared->channeledIncoming_.erase(tid);
}
{
std::lock_guard<std::mutex> lk(shared->channeledOutgoingMtx_);
shared->channeledOutgoing_.erase(tid);
}
- Manager::instance().dataTransfers->close(finalId);
});
}
@@ -977,7 +971,6 @@ DhtPeerConnector::requestConnection(
shared->channeledOutgoing_.erase(outgoingTransfers);
}
}
- Manager::instance().dataTransfers->close(tid);
});
});
// Cancel via DHT because we will use the channeled path
@@ -1080,17 +1073,9 @@ DhtPeerConnector::onIncomingConnection(const std::string& peer_id,
if (!shared)
return;
// Cancel incoming files
- DRing::DataTransferId internalId = 0;
- {
- std::lock_guard<std::mutex> lk(shared->channeledIncomingMtx_);
- auto it = shared->channeledIncoming_.find(tid);
- if (it != shared->channeledIncoming_.end())
- internalId = it->second->id();
- shared->channeledIncoming_.erase(tid);
- }
- if (internalId != 0) {
- Manager::instance().dataTransfers->close(internalId);
- }
+ // Note: erasing the channeled transfer will close the file via ftp_->close()
+ std::lock_guard<std::mutex> lk(shared->channeledIncomingMtx_);
+ shared->channeledIncoming_.erase(tid);
});
});
}
--
GitLab