Commit aa4f3abd authored by Sébastien Blin's avatar Sébastien Blin

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
parent 7950c1d3
...@@ -112,10 +112,9 @@ FtpServer::startNewFile() ...@@ -112,10 +112,9 @@ FtpServer::startNewFile()
void void
FtpServer::closeCurrentFile() FtpServer::closeCurrentFile()
{ {
if (out_.stream) { if (out_.stream && not closed_.exchange(true)) {
out_.stream->close(); out_.stream->close();
out_.stream.reset(); out_.stream.reset();
closed_ = true;
} }
} }
...@@ -123,8 +122,7 @@ bool ...@@ -123,8 +122,7 @@ bool
FtpServer::read(std::vector<uint8_t>& buffer) const FtpServer::read(std::vector<uint8_t>& buffer) const
{ {
if (!out_.stream) { if (!out_.stream) {
if (closed_) { if (closed_.exchange(false)) {
closed_ = false;
if (rx_ < fileSize_) { if (rx_ < fileSize_) {
buffer.resize(4); buffer.resize(4);
buffer[0] = 'N'; buffer[0] = 'N';
......
...@@ -81,7 +81,7 @@ private: ...@@ -81,7 +81,7 @@ private:
std::stringstream headerStream_; std::stringstream headerStream_;
std::string displayName_; std::string displayName_;
std::array<char, 1000> line_; std::array<char, 1000> line_;
mutable bool closed_ {false}; mutable std::atomic_bool closed_ {false};
mutable bool go_ {false}; mutable bool go_ {false};
FtpState state_ {FtpState::PARSE_HEADERS}; FtpState state_ {FtpState::PARSE_HEADERS};
......
...@@ -40,6 +40,7 @@ ChanneledOutgoingTransfer::~ChanneledOutgoingTransfer() ...@@ -40,6 +40,7 @@ ChanneledOutgoingTransfer::~ChanneledOutgoingTransfer()
channel_->setOnRecv({}); channel_->setOnRecv({});
file_->setOnRecv({}); file_->setOnRecv({});
channel_->shutdown(); channel_->shutdown();
file_->close();
} }
std::string std::string
...@@ -99,14 +100,8 @@ ChanneledIncomingTransfer::~ChanneledIncomingTransfer() ...@@ -99,14 +100,8 @@ ChanneledIncomingTransfer::~ChanneledIncomingTransfer()
{ {
channel_->setOnRecv({}); channel_->setOnRecv({});
channel_->shutdown(); channel_->shutdown();
}
DRing::DataTransferId
ChanneledIncomingTransfer::id() const
{
if (ftp_) if (ftp_)
return ftp_->getId(); ftp_->close();
return 0;
} }
} // namespace jami } // namespace jami
\ No newline at end of file
...@@ -53,7 +53,6 @@ public: ...@@ -53,7 +53,6 @@ public:
const std::shared_ptr<FtpServer>& ftp, const std::shared_ptr<FtpServer>& ftp,
OnStateChangedCb&& cb); OnStateChangedCb&& cb);
~ChanneledIncomingTransfer(); ~ChanneledIncomingTransfer();
DRing::DataTransferId id() const;
private: private:
std::shared_ptr<FtpServer> ftp_; std::shared_ptr<FtpServer> ftp_;
......
...@@ -837,20 +837,14 @@ DhtPeerConnector::Impl::cancelChanneled(const std::string& peerId, const DRing:: ...@@ -837,20 +837,14 @@ DhtPeerConnector::Impl::cancelChanneled(const std::string& peerId, const DRing::
if (!shared) if (!shared)
return; return;
// Cancel outgoing files // Cancel outgoing files
DRing::DataTransferId finalId = tid;
{ {
std::lock_guard<std::mutex> lk(shared->channeledIncomingMtx_); std::lock_guard<std::mutex> lk(shared->channeledIncomingMtx_);
auto it = shared->channeledIncoming_.find(tid); auto it = shared->channeledIncoming_.erase(tid);
if (it != shared->channeledIncoming_.end()) {
finalId = it->second->id();
}
shared->channeledIncoming_.erase(tid);
} }
{ {
std::lock_guard<std::mutex> lk(shared->channeledOutgoingMtx_); std::lock_guard<std::mutex> lk(shared->channeledOutgoingMtx_);
shared->channeledOutgoing_.erase(tid); shared->channeledOutgoing_.erase(tid);
} }
Manager::instance().dataTransfers->close(finalId);
}); });
} }
...@@ -977,7 +971,6 @@ DhtPeerConnector::requestConnection( ...@@ -977,7 +971,6 @@ DhtPeerConnector::requestConnection(
shared->channeledOutgoing_.erase(outgoingTransfers); shared->channeledOutgoing_.erase(outgoingTransfers);
} }
} }
Manager::instance().dataTransfers->close(tid);
}); });
}); });
// Cancel via DHT because we will use the channeled path // Cancel via DHT because we will use the channeled path
...@@ -1080,17 +1073,9 @@ DhtPeerConnector::onIncomingConnection(const std::string& peer_id, ...@@ -1080,17 +1073,9 @@ DhtPeerConnector::onIncomingConnection(const std::string& peer_id,
if (!shared) if (!shared)
return; return;
// Cancel incoming files // Cancel incoming files
DRing::DataTransferId internalId = 0; // Note: erasing the channeled transfer will close the file via ftp_->close()
{ std::lock_guard<std::mutex> lk(shared->channeledIncomingMtx_);
std::lock_guard<std::mutex> lk(shared->channeledIncomingMtx_); shared->channeledIncoming_.erase(tid);
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);
}
}); });
}); });
} }
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment