loolwsd/ChildProcessSession.cpp | 56 ++++++++++++++++++++++------------------ loolwsd/ChildProcessSession.hpp | 8 +++++ loolwsd/LOOLKit.cpp | 19 +++++++------ 3 files changed, 49 insertions(+), 34 deletions(-)
New commits: commit 55f857e17cf8a7989a5b36d874d3c8cb43d6ebfd Author: Ashod Nakashian <[email protected]> Date: Sun Jan 10 22:52:00 2016 -0500 loolwsd: Poco::Mutex -> std::mutex and callback locks on session Change-Id: I9c7d16352110566e5fc31c280784ded30cd3a9be Reviewed-on: https://gerrit.libreoffice.org/21333 Reviewed-by: Ashod Nakashian <[email protected]> Tested-by: Ashod Nakashian <[email protected]> diff --git a/loolwsd/ChildProcessSession.cpp b/loolwsd/ChildProcessSession.cpp index 1fb17ca..7db9964 100644 --- a/loolwsd/ChildProcessSession.cpp +++ b/loolwsd/ChildProcessSession.cpp @@ -38,7 +38,7 @@ using Poco::ProcessHandle; using Poco::StringTokenizer; using Poco::URI; -Poco::Mutex ChildProcessSession::_mutex; +std::mutex ChildProcessSession::_mutex; ChildProcessSession::ChildProcessSession(const std::string& id, std::shared_ptr<Poco::Net::WebSocket> ws, @@ -64,7 +64,7 @@ ChildProcessSession::~ChildProcessSession() { Log::info("~ChildProcessSession dtor [" + getName() + "]."); - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); @@ -146,7 +146,7 @@ bool ChildProcessSession::_handleInput(const char *buffer, int length) tokens[0] == "saveas"); { - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); @@ -232,10 +232,10 @@ bool ChildProcessSession::loadDocument(const char * /*buffer*/, int /*length*/, assert(!_docURL.empty()); assert(!_jailedFilePath.empty()); - Poco::Mutex::ScopedLock lock(_mutex); - _loKitDocument = _onLoad(getId(), _jailedFilePath); + std::unique_lock<std::mutex> lock(_mutex); + if (_multiView) _viewId = _loKitDocument->pClass->getView(_loKitDocument); @@ -257,7 +257,7 @@ bool ChildProcessSession::loadDocument(const char * /*buffer*/, int /*length*/, } // Respond by the document status, which has no arguments. - if (!getStatus(nullptr, 0)) + if (!getStatus_Impl(nullptr, 0)) return false; Log::info("Loaded session " + getId()); @@ -275,7 +275,7 @@ void ChildProcessSession::sendFontRendering(const char* /*buffer*/, int /*length return; } - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); @@ -306,18 +306,24 @@ void ChildProcessSession::sendFontRendering(const char* /*buffer*/, int /*length sendBinaryFrame(output.data(), output.size()); } -bool ChildProcessSession::getStatus(const char* /*buffer*/, int /*length*/) +bool ChildProcessSession::getStatus(const char* buffer, int length) { - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); + return getStatus_Impl(buffer, length); +} + +bool ChildProcessSession::getStatus_Impl(const char* /*buffer*/, int /*length*/) +{ std::string status = "status: " + LOKitHelper::documentStatus(_loKitDocument); StringTokenizer tokens(status, " ", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM); if (!getTokenString(tokens[1], "type", _docType)) { Log::error("failed to get document type from status [" + status + "]."); + return false; } sendTextFrame(status); @@ -334,7 +340,7 @@ bool ChildProcessSession::getCommandValues(const char* /*buffer*/, int /*length* return false; } - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); @@ -345,7 +351,7 @@ bool ChildProcessSession::getCommandValues(const char* /*buffer*/, int /*length* bool ChildProcessSession::getPartPageRectangles(const char* /*buffer*/, int /*length*/) { - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); @@ -383,7 +389,7 @@ void ChildProcessSession::sendTile(const char* /*buffer*/, int /*length*/, Strin return; } - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); @@ -432,7 +438,7 @@ bool ChildProcessSession::clientZoom(const char* /*buffer*/, int /*length*/, Str return false; } - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); @@ -466,7 +472,7 @@ bool ChildProcessSession::downloadAs(const char* /*buffer*/, int /*length*/, Str const auto tmpDir = Util::createRandomDir(JailedDocumentRoot); const auto url = JailedDocumentRoot + tmpDir + "/" + name; - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); //TODO: Cleanup the file after downloading. _loKitDocument->pClass->saveAs(_loKitDocument, url.c_str(), @@ -495,7 +501,7 @@ bool ChildProcessSession::getTextSelection(const char* /*buffer*/, int /*length* return false; } - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); @@ -519,7 +525,7 @@ bool ChildProcessSession::paste(const char* /*buffer*/, int /*length*/, StringTo data = Poco::cat(std::string(" "), tokens.begin() + 2, tokens.end()).substr(strlen("data=")); - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); @@ -541,7 +547,7 @@ bool ChildProcessSession::insertFile(const char* /*buffer*/, int /*length*/, Str return false; } - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (type == "graphic") { @@ -582,7 +588,7 @@ bool ChildProcessSession::keyEvent(const char* /*buffer*/, int /*length*/, Strin if (keycode == (KEY_CTRL | KEY_W)) return true; - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); @@ -612,7 +618,7 @@ bool ChildProcessSession::mouseEvent(const char* /*buffer*/, int /*length*/, Str return false; } - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); @@ -630,7 +636,7 @@ bool ChildProcessSession::unoCommand(const char* /*buffer*/, int /*length*/, Str return false; } - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); @@ -667,7 +673,7 @@ bool ChildProcessSession::selectText(const char* /*buffer*/, int /*length*/, Str return false; } - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); @@ -693,7 +699,7 @@ bool ChildProcessSession::selectGraphic(const char* /*buffer*/, int /*length*/, return false; } - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); @@ -711,7 +717,7 @@ bool ChildProcessSession::resetSelection(const char* /*buffer*/, int /*length*/, return false; } - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); @@ -742,7 +748,7 @@ bool ChildProcessSession::saveAs(const char* /*buffer*/, int /*length*/, StringT } } - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); @@ -780,7 +786,7 @@ bool ChildProcessSession::setPage(const char* /*buffer*/, int /*length*/, String return false; } - Poco::Mutex::ScopedLock lock(_mutex); + std::unique_lock<std::mutex> lock(_mutex); if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); diff --git a/loolwsd/ChildProcessSession.hpp b/loolwsd/ChildProcessSession.hpp index 4313d81..61ae15a 100644 --- a/loolwsd/ChildProcessSession.hpp +++ b/loolwsd/ChildProcessSession.hpp @@ -10,6 +10,8 @@ #ifndef INCLUDED_LOOLCHILDPROCESSSESSION_HPP #define INCLUDED_LOOLCHILDPROCESSSESSION_HPP +#include <mutex> + #define LOK_USE_UNSTABLE_API #include <LibreOfficeKit/LibreOfficeKit.h> @@ -50,6 +52,8 @@ public: LibreOfficeKitDocument *getLoKitDocument() const { return _loKitDocument; } + std::unique_lock<std::mutex> lock() { return std::unique_lock<std::mutex>(_mutex); } + protected: virtual bool loadDocument(const char *buffer, int length, Poco::StringTokenizer& tokens) override; @@ -72,6 +76,7 @@ public: bool saveAs(const char *buffer, int length, Poco::StringTokenizer& tokens); bool setClientPart(const char *buffer, int length, Poco::StringTokenizer& tokens); bool setPage(const char *buffer, int length, Poco::StringTokenizer& tokens); + bool getStatus_Impl(const char* buffer, int length); private: @@ -80,7 +85,6 @@ private: private: LibreOfficeKitDocument *_loKitDocument; std::string _docType; - static Poco::Mutex _mutex; const bool _multiView; LibreOfficeKit *_loKit; const std::string _jailId; @@ -89,6 +93,8 @@ private: int _clientPart; std::function<LibreOfficeKitDocument*(const std::string&, const std::string&)> _onLoad; std::function<void(const std::string&)> _onUnload; + + static std::mutex _mutex; }; #endif diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp index f6fa52f..6f4d2e1 100644 --- a/loolwsd/LOOLKit.cpp +++ b/loolwsd/LOOLKit.cpp @@ -139,6 +139,8 @@ public: void callback(const int nType, const std::string& rPayload, std::shared_ptr<ChildProcessSession> pSession) { + auto lock = pSession->lock(); + Log::trace() << "Callback [" << pSession->getViewId() << "] " << callbackTypeToString(nType) << " [" << rPayload << "]." << Log::end; @@ -623,11 +625,11 @@ private: /// Load a document (or view) and register callbacks. LibreOfficeKitDocument* onLoad(const std::string& sessionId, const std::string& uri) { + std::unique_lock<std::mutex> lock(_mutex); + Log::info("Session " + sessionId + " is loading. " + std::to_string(_clientViews) + " views loaded."); const unsigned intSessionId = Util::decodeId(sessionId); - - std::unique_lock<std::mutex> lock(_mutex); const auto it = _connections.find(intSessionId); if (it == _connections.end() || !it->second) { @@ -635,8 +637,6 @@ private: return nullptr; } - lock.unlock(); - if (_loKitDocument == nullptr) { Log::info("Loading new document from URI: [" + uri + "] for session [" + sessionId + "]."); @@ -644,11 +644,16 @@ private: if ( LIBREOFFICEKIT_HAS(_loKit, registerCallback)) _loKit->pClass->registerCallback(_loKit, DocumentCallback, this); + // documentLoad will trigger callback, which needs to take the lock. + lock.unlock(); if ((_loKitDocument = _loKit->pClass->documentLoad(_loKit, uri.c_str())) == nullptr) { Log::error("Failed to load: " + uri + ", error: " + _loKit->pClass->getError(_loKit)); return nullptr; } + + // Retake the lock. + lock.lock(); } if (_multiView) @@ -673,11 +678,11 @@ private: void onUnload(const std::string& sessionId) { + std::unique_lock<std::mutex> lock(_mutex); + Log::info("Session " + sessionId + " is unloading. " + std::to_string(_clientViews - 1) + " views left."); const unsigned intSessionId = Util::decodeId(sessionId); - - std::unique_lock<std::mutex> lock(_mutex); const auto it = _connections.find(intSessionId); if (it == _connections.end() || !it->second) { @@ -685,8 +690,6 @@ private: return; } - lock.unlock(); - --_clientViews; if (_multiView && _loKitDocument) _______________________________________________ Libreoffice-commits mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
