configure.ac | 2 +- wsd/ClientSession.cpp | 5 ----- wsd/DocumentBroker.cpp | 44 ++++++++++++++++++++++++++++++++++++++------ wsd/DocumentBroker.hpp | 24 ++++++++++++++++++++---- wsd/LOOLWSD.cpp | 44 ++++++++++++++++++++++++++++++-------------- 5 files changed, 89 insertions(+), 30 deletions(-)
New commits: commit 49c65bbc306cabf71323cb9f5e8b47528cb76f2f Author: Samuel Mehrbrodt <samuel.mehrbr...@cib.de> AuthorDate: Tue Apr 14 09:10:12 2020 +0200 Commit: Samuel Mehrbrodt <samuel.mehrbr...@cib.de> CommitDate: Tue Apr 14 09:10:12 2020 +0200 Release 6.2.7.0 diff --git a/configure.ac b/configure.ac index 5aebd9ba5..9a8a64c96 100644 --- a/configure.ac +++ b/configure.ac @@ -3,7 +3,7 @@ AC_PREREQ([2.63]) -AC_INIT([loolwsd], [6.2.6.0], [libreoffice@lists.freedesktop.org]) +AC_INIT([loolwsd], [6.2.7.0], [libreoffice@lists.freedesktop.org]) LT_INIT([shared, disable-static, dlopen]) AM_INIT_AUTOMAKE([1.10 subdir-objects tar-pax -Wno-portability]) commit c6e72be9fcdf3a3439266f4e3b7a4f87565f2c3c Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Tue May 21 19:50:17 2019 +0100 Commit: Samuel Mehrbrodt <samuel.mehrbr...@cib.de> CommitDate: Tue Apr 14 08:25:11 2020 +0200 tdf#123482 - cleanup convert-to folder even more reliably. Problems could occur if exceptiosn thrown when parsing the input stream. Change-Id: Id82b3816450194164fc2093554c730b4a94acef1 Reviewed-on: https://gerrit.libreoffice.org/72695 Reviewed-by: Michael Meeks <michael.me...@collabora.com> Tested-by: Michael Meeks <michael.me...@collabora.com> (cherry picked from commit 1845ed42af661dbb7b10a2fb4686ce2c8ef7e521) diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 86524325f..61691133f 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -1880,13 +1880,18 @@ ConvertToBroker::ConvertToBroker(const std::string& uri, ConvertToBroker::~ConvertToBroker() { NumConverters--; - if (!_uriOrig.empty()) + removeFile(_uriOrig); +} + +void ConvertToBroker::removeFile(const std::string &uriOrig) +{ + if (!uriOrig.empty()) { // Remove source file and directory - Poco::Path path = _uriOrig; + Poco::Path path = uriOrig; Poco::File(path).remove(); Poco::File(path.makeParent()).remove(); - FileUtil::removeFile(_uriOrig); + FileUtil::removeFile(uriOrig); } } diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp index 4e628af12..b2c7543ba 100644 --- a/wsd/DocumentBroker.hpp +++ b/wsd/DocumentBroker.hpp @@ -484,6 +484,9 @@ public: /// How many live conversions are running. static size_t getInstanceCount(); + + /// Cleanup path and its parent + static void removeFile(const std::string &uri); }; #endif diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index 4878e116e..5415224fa 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -570,6 +570,9 @@ class ConvertToPartHandler : public PartHandler public: std::string getFilename() const { return _filename; } + /// Afterwards someone else is responsible for cleaning that up. + void takeFile() { _filename.clear(); } + ConvertToPartHandler(bool convertTo = false) : _convertTo(convertTo) { @@ -577,6 +580,11 @@ public: virtual ~ConvertToPartHandler() { + if (!_filename.empty()) + { + LOG_TRC("Remove un-handled temporary file '" << _filename << "'"); + ConvertToBroker::removeFile(_filename); + } } virtual void handlePart(const MessageHeader& header, std::istream& stream) override @@ -2362,6 +2370,7 @@ private: LOG_DBG("New DocumentBroker for docKey [" << docKey << "]."); auto docBroker = std::make_shared<ConvertToBroker>(fromPath, uriPublic, docKey); + handler.takeFile(); cleanupDocBrokers(); commit dab5603fc6bdb9ffbc576c9ae693724484f31b37 Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Tue Mar 12 15:41:54 2019 +0100 Commit: Samuel Mehrbrodt <samuel.mehrbr...@cib.de> CommitDate: Tue Apr 14 08:23:49 2020 +0200 Don't count convert-to connections vs. the document count. Change-Id: I350905fb98c503ae8f22a377e4af5cbcb9f3c52d (cherry picked from commit d8bb92cdcf83216ae5a56b8602cbd9f7c72b4975) diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 2d00ac568..86524325f 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -1862,8 +1862,24 @@ void DocumentBroker::getIOStats(uint64_t &sent, uint64_t &recv) } } +static std::atomic<size_t> NumConverters; + +size_t ConvertToBroker::getInstanceCount() +{ + return NumConverters; +} + +ConvertToBroker::ConvertToBroker(const std::string& uri, + const Poco::URI& uriPublic, + const std::string& docKey) + : DocumentBroker(uri, uriPublic, docKey) +{ + NumConverters++; +} + ConvertToBroker::~ConvertToBroker() { + NumConverters--; if (!_uriOrig.empty()) { // Remove source file and directory diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp index ddf5ec10b..4e628af12 100644 --- a/wsd/DocumentBroker.hpp +++ b/wsd/DocumentBroker.hpp @@ -479,11 +479,11 @@ public: /// Construct DocumentBroker with URI and docKey ConvertToBroker(const std::string& uri, const Poco::URI& uriPublic, - const std::string& docKey) - : DocumentBroker(uri, uriPublic, docKey) - { - } + const std::string& docKey); virtual ~ConvertToBroker(); + + /// How many live conversions are running. + static size_t getInstanceCount(); }; #endif diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index c8b20552c..4878e116e 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -254,8 +254,8 @@ inline void shutdownLimitReached(WebSocketHandler& ws) inline void checkSessionLimitsAndWarnClients() { #ifndef MOBILEAPP - - if (DocBrokers.size() > LOOLWSD::MaxDocuments || LOOLWSD::NumConnections >= LOOLWSD::MaxConnections) + size_t docBrokerCount = DocBrokers.size() - ConvertToBroker::getInstanceCount(); + if (docBrokerCount > LOOLWSD::MaxDocuments || LOOLWSD::NumConnections >= LOOLWSD::MaxConnections) { const std::string info = Poco::format(PAYLOAD_INFO_LIMIT_REACHED, LOOLWSD::MaxDocuments, LOOLWSD::MaxConnections); LOG_INF("Sending client 'limitreached' message: " << info); @@ -2931,6 +2931,7 @@ public: << "[ " << DocBrokers.size() << " ]:\n"; for (auto &i : DocBrokers) i.second->dumpState(os); + os << "Converter count: " << ConvertToBroker::getInstanceCount() << "\n"; Socket::InhibitThreadChecks = false; SocketPoll::InhibitThreadChecks = false; commit 3d5ccd6186e6b6ee55da938b8ac0c9b8068779e4 Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Fri Mar 1 22:25:44 2019 +0100 Commit: Samuel Mehrbrodt <samuel.mehrbr...@cib.de> CommitDate: Tue Apr 14 08:17:42 2020 +0200 tdf#123482 - cleanup convert-to folder more reliably. Change-Id: I029bb4136984e05485e462c92da80b92b00fdebc (cherry picked from commit 2d473222e4cad399b131345395d6506b26e0e134) diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp index 102fc0325..e3cb678e5 100644 --- a/wsd/ClientSession.cpp +++ b/wsd/ClientSession.cpp @@ -887,11 +887,6 @@ bool ClientSession::handleKitToClientMessage(const char* buffer, const int lengt // Now terminate. docBroker->stop("Finished saveas handler."); - - // Remove file and directory - Poco::Path path = docBroker->getDocKey(); - Poco::File(path).remove(); - Poco::File(path.makeParent()).remove(); } return true; diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 934c2ce0b..2d00ac568 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -37,6 +37,7 @@ #include <common/Message.hpp> #include <common/Protocol.hpp> #include <common/Unit.hpp> +#include <common/FileUtil.hpp> #include <sys/types.h> #include <sys/wait.h> @@ -1861,6 +1862,18 @@ void DocumentBroker::getIOStats(uint64_t &sent, uint64_t &recv) } } +ConvertToBroker::~ConvertToBroker() +{ + if (!_uriOrig.empty()) + { + // Remove source file and directory + Poco::Path path = _uriOrig; + Poco::File(path).remove(); + Poco::File(path.makeParent()).remove(); + FileUtil::removeFile(_uriOrig); + } +} + void DocumentBroker::dumpState(std::ostream& os) { std::unique_lock<std::mutex> lock(_mutex); diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp index 15d3b95b7..ddf5ec10b 100644 --- a/wsd/DocumentBroker.hpp +++ b/wsd/DocumentBroker.hpp @@ -223,7 +223,7 @@ public: const Poco::URI& uriPublic, const std::string& docKey); - ~DocumentBroker(); + virtual ~DocumentBroker(); /// Start processing events void startThread(); @@ -402,8 +402,9 @@ private: /// Sum the I/O stats from all connected sessions void getIOStats(uint64_t &sent, uint64_t &recv); -private: +protected: const std::string _uriOrig; +private: const Poco::URI _uriPublic; /// URL-based key. May be repeated during the lifetime of WSD. const std::string _docKey; @@ -472,6 +473,19 @@ private: static std::atomic<unsigned> DocBrokerId; }; +class ConvertToBroker : public DocumentBroker +{ +public: + /// Construct DocumentBroker with URI and docKey + ConvertToBroker(const std::string& uri, + const Poco::URI& uriPublic, + const std::string& docKey) + : DocumentBroker(uri, uriPublic, docKey) + { + } + virtual ~ConvertToBroker(); +}; + #endif /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index a86a996a5..c8b20552c 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -593,6 +593,7 @@ public: if (!params.has("filename")) return; + // FIXME: needs wrapping - until then - keep in sync with ~ConvertToBroker Path tempPath = _convertTo? Path::forDirectory(Poco::TemporaryFile::tempName("/tmp/convert-to") + "/") : Path::forDirectory(Poco::TemporaryFile::tempName() + "/"); File(tempPath).createDirectories(); @@ -2360,7 +2361,7 @@ private: std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex); LOG_DBG("New DocumentBroker for docKey [" << docKey << "]."); - auto docBroker = std::make_shared<DocumentBroker>(fromPath, uriPublic, docKey); + auto docBroker = std::make_shared<ConvertToBroker>(fromPath, uriPublic, docKey); cleanupDocBrokers(); commit 75bd2359807fa8a6e7c3d14cb22b937c326790eb Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Fri Mar 1 22:03:35 2019 +0100 Commit: Samuel Mehrbrodt <samuel.mehrbr...@cib.de> CommitDate: Tue Apr 14 08:15:49 2020 +0200 Simpify DocumentBroker constructor. Change-Id: I0bf29df9316b129d34862c7464bb6636d42a850d (cherry picked from commit 51b72b04a9e0c39df525caeb0744ca44a8ba0143) diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 661dbb2a2..934c2ce0b 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -145,13 +145,11 @@ std::atomic<unsigned> DocumentBroker::DocBrokerId(1); DocumentBroker::DocumentBroker(const std::string& uri, const Poco::URI& uriPublic, - const std::string& docKey, - const std::string& childRoot) : + const std::string& docKey) : _uriOrig(uri), _uriPublic(uriPublic), _docKey(docKey), _docId(Util::encodeId(DocBrokerId++, 3)), - _childRoot(childRoot), _cacheRoot(getCachePath(uriPublic.toString())), _documentChangedInStorage(false), _lastSaveTime(std::chrono::steady_clock::now()), @@ -171,10 +169,10 @@ DocumentBroker::DocumentBroker(const std::string& uri, _debugRenderedTileCount(0) { assert(!_docKey.empty()); - assert(!_childRoot.empty()); + assert(!LOOLWSD::ChildRoot.empty()); LOG_INF("DocumentBroker [" << LOOLWSD::anonymizeUrl(_uriPublic.toString()) << - "] created with docKey [" << _docKey << "] and root [" << _childRoot << "]"); + "] created with docKey [" << _docKey << "]"); } void DocumentBroker::startThread() @@ -1051,7 +1049,7 @@ bool DocumentBroker::sendUnoSave(const std::string& sessionId, bool dontTerminat std::string DocumentBroker::getJailRoot() const { assert(!_jailId.empty()); - return Poco::Path(_childRoot, _jailId).toString(); + return Poco::Path(LOOLWSD::ChildRoot, _jailId).toString(); } size_t DocumentBroker::addSession(const std::shared_ptr<ClientSession>& session) diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp index 45f37edd1..15d3b95b7 100644 --- a/wsd/DocumentBroker.hpp +++ b/wsd/DocumentBroker.hpp @@ -221,8 +221,7 @@ public: /// Construct DocumentBroker with URI, docKey, and root path. DocumentBroker(const std::string& uri, const Poco::URI& uriPublic, - const std::string& docKey, - const std::string& childRoot); + const std::string& docKey); ~DocumentBroker(); diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index 115441065..a86a996a5 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -1696,7 +1696,7 @@ static std::shared_ptr<DocumentBroker> findOrCreateDocBroker(WebSocketHandler& w // Set the one we just created. LOG_DBG("New DocumentBroker for docKey [" << docKey << "]."); - docBroker = std::make_shared<DocumentBroker>(uri, uriPublic, docKey, LOOLWSD::ChildRoot); + docBroker = std::make_shared<DocumentBroker>(uri, uriPublic, docKey); DocBrokers.emplace(docKey, docBroker); LOG_TRC("Have " << DocBrokers.size() << " DocBrokers after inserting [" << docKey << "]."); } @@ -2360,7 +2360,7 @@ private: std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex); LOG_DBG("New DocumentBroker for docKey [" << docKey << "]."); - auto docBroker = std::make_shared<DocumentBroker>(fromPath, uriPublic, docKey, LOOLWSD::ChildRoot); + auto docBroker = std::make_shared<DocumentBroker>(fromPath, uriPublic, docKey); cleanupDocBrokers(); commit 6d7d4ed4662be8b9c8607358e81ac1fed051fa74 Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Fri Mar 1 18:59:40 2019 +0100 Commit: Samuel Mehrbrodt <samuel.mehrbr...@cib.de> CommitDate: Tue Apr 14 08:04:34 2020 +0200 Avoid using un-necessary reference. Change-Id: Id5a9fed8fb790f2af8facac119e9e0da476b1e47 (cherry picked from commit b255ce528bb6c7d037c99ff612a8efba92182abd) diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index a4a5533fe..115441065 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -558,18 +558,24 @@ std::shared_ptr<ChildProcess> getNewChild_Blocks( #ifndef MOBILEAPP -/// Handles the filename part of the convert-to POST request payload. +/// Handles the filename part of the convert-to POST request payload, +/// Also owns the file - cleaning it up when destroyed. class ConvertToPartHandler : public PartHandler { - std::string& _filename; + std::string _filename; /// Is it really a convert-to, ie. use an especially formed path? bool _convertTo; public: - ConvertToPartHandler(std::string& filename, bool convertTo = false) - : _filename(filename) - , _convertTo(convertTo) + std::string getFilename() const { return _filename; } + + ConvertToPartHandler(bool convertTo = false) + : _convertTo(convertTo) + { + } + + virtual ~ConvertToPartHandler() { } @@ -2318,8 +2324,7 @@ private: StringTokenizer tokens(request.getURI(), "/?"); if (tokens.count() > 2 && tokens[2] == "convert-to") { - std::string fromPath; - ConvertToPartHandler handler(fromPath, /*convertTo =*/ true); + ConvertToPartHandler handler(/*convertTo =*/ true); HTMLForm form(request, message, handler); std::string format = (form.has("format") ? form.get("format") : ""); @@ -2343,6 +2348,7 @@ private: format = tokens[3]; bool sent = false; + std::string fromPath = handler.getFilename(); LOG_INF("Conversion request for URI [" << fromPath << "] format [" << format << "]."); if (!fromPath.empty() && !format.empty()) { @@ -2428,8 +2434,7 @@ private: { LOG_INF("Insert file request."); - std::string tmpPath; - ConvertToPartHandler handler(tmpPath); + ConvertToPartHandler handler; HTMLForm form(request, message, handler); if (form.has("childid") && form.has("name")) @@ -2459,7 +2464,7 @@ private: + JAILED_DOCUMENT_ROOT + "insertfile"; File(dirPath).createDirectories(); std::string fileName = dirPath + "/" + form.get("name"); - File(tmpPath).moveTo(fileName); + File(handler.getFilename()).moveTo(fileName); response.setContentLength(0); socket->send(response); return; _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits