[Libreoffice-commits] online.git: loolwsd/ChildProcessSession.cpp loolwsd/ChildProcessSession.hpp loolwsd/LOOLKit.cpp
Ashod Nakashian
ashod.nakashian at collabora.co.uk
Sun Jan 10 20:12:29 PST 2016
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 <ashod.nakashian at collabora.co.uk>
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 <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
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)
More information about the Libreoffice-commits
mailing list