summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--kit/ChildSession.cpp169
-rw-r--r--kit/ChildSession.hpp4
-rw-r--r--kit/Kit.cpp20
-rw-r--r--test/WhiteBoxTests.cpp5
4 files changed, 50 insertions, 148 deletions
diff --git a/kit/ChildSession.cpp b/kit/ChildSession.cpp
index d8255442e..60cf4c087 100644
--- a/kit/ChildSession.cpp
+++ b/kit/ChildSession.cpp
@@ -122,7 +122,6 @@ bool ChildSession::_handleInput(const char *buffer, int length)
// Client is getting active again.
// Send invalidation and other sync-up messages.
std::unique_lock<std::recursive_mutex> lock(Mutex); //TODO: Move to top of function?
- std::unique_lock<std::mutex> lockLokDoc(_docManager.getDocumentMutex());
getLOKitDocument()->setView(_viewId);
@@ -133,8 +132,6 @@ bool ChildSession::_handleInput(const char *buffer, int length)
// Notify all views about updated view info
_docManager.notifyViewInfo();
- lockLokDoc.unlock();
-
if (getLOKitDocument()->getDocumentType() != LOK_DOCTYPE_TEXT)
{
sendTextFrame("curpart: part=" + std::to_string(curPart));
@@ -456,13 +453,9 @@ bool ChildSession::uploadSignedDocument(const char* buffer, int length, const st
const Poco::Path filenameParam(filename);
const std::string url = JAILED_DOCUMENT_ROOT + tmpDir + "/" + filenameParam.getFileName();
- {
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
- getLOKitDocument()->saveAs(url.c_str(),
- filetype.empty() ? nullptr : filetype.c_str(),
- nullptr);
- }
+ getLOKitDocument()->saveAs(url.c_str(),
+ filetype.empty() ? nullptr : filetype.c_str(),
+ nullptr);
Authorization authorization(Authorization::Type::Token, token);
Poco::URI uriObject(wopiUrl + "/" + filename + "/contents");
@@ -573,8 +566,6 @@ bool ChildSession::loadDocument(const char * /*buffer*/, int /*length*/, const s
LOG_INF("Created new view with viewid: [" << _viewId << "] for username: [" <<
getUserNameAnonym() << "] in session: [" << getId() << "].");
- std::unique_lock<std::mutex> lockLokDoc(_docManager.getDocumentMutex());
-
getLOKitDocument()->setView(_viewId);
_docType = LOKitHelper::getDocumentTypeAsString(getLOKitDocument()->get());
@@ -638,13 +629,9 @@ bool ChildSession::sendFontRendering(const char* /*buffer*/, int /*length*/, con
int width = 0, height = 0;
unsigned char* ptrFont = nullptr;
- {
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
- getLOKitDocument()->setView(_viewId);
+ getLOKitDocument()->setView(_viewId);
- ptrFont = getLOKitDocument()->renderFont(decodedFont.c_str(), decodedChar.c_str(), &width, &height);
- }
+ ptrFont = getLOKitDocument()->renderFont(decodedFont.c_str(), decodedChar.c_str(), &width, &height);
LOG_TRC("renderFont [" << font << "] rendered in " << (timestamp.elapsed()/1000.) << "ms");
@@ -671,13 +658,10 @@ bool ChildSession::sendFontRendering(const char* /*buffer*/, int /*length*/, con
bool ChildSession::getStatus(const char* /*buffer*/, int /*length*/)
{
std::string status;
- {
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
- getLOKitDocument()->setView(_viewId);
+ getLOKitDocument()->setView(_viewId);
- status = LOKitHelper::documentStatus(getLOKitDocument()->get());
- }
+ status = LOKitHelper::documentStatus(getLOKitDocument()->get());
if (status.empty())
{
@@ -731,8 +715,6 @@ bool ChildSession::getCommandValues(const char* /*buffer*/, int /*length*/, cons
return false;
}
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
getLOKitDocument()->setView(_viewId);
if (command == ".uno:DocumentRepair")
@@ -775,8 +757,6 @@ bool ChildSession::clientZoom(const char* /*buffer*/, int /*length*/, const std:
return false;
}
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
getLOKitDocument()->setView(_viewId);
getLOKitDocument()->setClientZoom(tilePixelWidth, tilePixelHeight, tileTwipWidth, tileTwipHeight);
@@ -800,8 +780,6 @@ bool ChildSession::clientVisibleArea(const char* /*buffer*/, int /*length*/, con
return false;
}
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
getLOKitDocument()->setView(_viewId);
getLOKitDocument()->setClientVisibleArea(x, y, width, height);
@@ -828,8 +806,6 @@ bool ChildSession::outlineState(const char* /*buffer*/, int /*length*/, const st
bool column = type == "column";
bool hidden = state == "hidden";
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
getLOKitDocument()->setView(_viewId);
getLOKitDocument()->setOutlineState(column, level, index, hidden);
@@ -875,17 +851,13 @@ bool ChildSession::downloadAs(const char* /*buffer*/, int /*length*/, const std:
const std::string nameAnonym = anonymizeUrl(name);
const std::string urlAnonym = JAILED_DOCUMENT_ROOT + tmpDir + "/" + Poco::Path(nameAnonym).getFileName();
- {
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
- LOG_DBG("Calling LOK's downloadAs with: url='" << urlAnonym << "', format='" <<
- (format.empty() ? "(nullptr)" : format.c_str()) << "', ' filterOptions=" <<
- (filterOptions.empty() ? "(nullptr)" : filterOptions.c_str()) << "'.");
+ LOG_DBG("Calling LOK's downloadAs with: url='" << urlAnonym << "', format='" <<
+ (format.empty() ? "(nullptr)" : format.c_str()) << "', ' filterOptions=" <<
+ (filterOptions.empty() ? "(nullptr)" : filterOptions.c_str()) << "'.");
- getLOKitDocument()->saveAs(url.c_str(),
- format.empty() ? nullptr : format.c_str(),
- filterOptions.empty() ? nullptr : filterOptions.c_str());
- }
+ getLOKitDocument()->saveAs(url.c_str(),
+ format.empty() ? nullptr : format.c_str(),
+ filterOptions.empty() ? nullptr : filterOptions.c_str());
sendTextFrame("downloadas: jail=" + _jailId + " dir=" + tmpDir + " name=" + name +
" port=" + std::to_string(ClientPortNumber) + " id=" + id);
@@ -901,13 +873,10 @@ bool ChildSession::getChildId()
std::string ChildSession::getTextSelectionInternal(const std::string& mimeType)
{
char* textSelection = nullptr;
- {
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
- getLOKitDocument()->setView(_viewId);
+ getLOKitDocument()->setView(_viewId);
- textSelection = getLOKitDocument()->getTextSelection(mimeType.c_str(), nullptr);
- }
+ textSelection = getLOKitDocument()->getTextSelection(mimeType.c_str(), nullptr);
std::string str(textSelection ? textSelection : "");
free(textSelection);
@@ -944,8 +913,6 @@ bool ChildSession::paste(const char* buffer, int length, const std::vector<std::
const int size = length - firstLine.size() - 1;
if (size > 0)
{
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
getLOKitDocument()->setView(_viewId);
getLOKitDocument()->paste(mimeType.c_str(), data, size);
@@ -1005,8 +972,6 @@ bool ChildSession::insertFile(const char* /*buffer*/, int /*length*/, const std:
"\"value\":\"" + url + "\""
"}}";
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
getLOKitDocument()->setView(_viewId);
LOG_TRC("Inserting graphic: '" << arguments.c_str() << "', '");
@@ -1037,7 +1002,6 @@ bool ChildSession::extTextInputEvent(const char* /*buffer*/, int /*length*/,
std::string decodedText;
URI::decode(text, decodedText);
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
getLOKitDocument()->setView(_viewId);
getLOKitDocument()->postWindowExtTextInputEvent(id, type, decodedText.c_str());
@@ -1092,7 +1056,6 @@ bool ChildSession::keyEvent(const char* /*buffer*/, int /*length*/,
return true;
}
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
getLOKitDocument()->setView(_viewId);
if (target == LokEventTargetEnum::Document)
getLOKitDocument()->postKeyEvent(type, charcode, keycode);
@@ -1132,8 +1095,6 @@ bool ChildSession::gestureEvent(const char* /*buffer*/, int /*length*/,
return false;
}
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
getLOKitDocument()->setView(_viewId);
getLOKitDocument()->postWindowGestureEvent(windowID, type.c_str(), x, y, offset);
@@ -1194,8 +1155,6 @@ bool ChildSession::mouseEvent(const char* /*buffer*/, int /*length*/,
return false;
}
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
getLOKitDocument()->setView(_viewId);
switch (target)
{
@@ -1226,8 +1185,6 @@ bool ChildSession::unoCommand(const char* /*buffer*/, int /*length*/, const std:
tokens[1] == ".uno:Redo" ||
Util::startsWith(tokens[1], "vnd.sun.star.script:"));
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
getLOKitDocument()->setView(_viewId);
if (tokens.size() == 2)
@@ -1270,8 +1227,6 @@ bool ChildSession::selectText(const char* /*buffer*/, int /*length*/, const std:
return false;
}
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
getLOKitDocument()->setView(_viewId);
getLOKitDocument()->setTextSelection(type, x, y);
@@ -1283,7 +1238,6 @@ bool ChildSession::renderWindow(const char* /*buffer*/, int /*length*/, const st
{
const unsigned winId = (tokens.size() > 1 ? std::stoul(tokens[1]) : 0);
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
getLOKitDocument()->setView(_viewId);
int startX = 0, startY = 0;
@@ -1354,7 +1308,6 @@ bool ChildSession::sendWindowCommand(const char* /*buffer*/, int /*length*/, con
{
const unsigned winId = (tokens.size() > 1 ? std::stoul(tokens[1]) : 0);
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
getLOKitDocument()->setView(_viewId);
if (tokens.size() > 2 && tokens[2] == "close")
@@ -1529,11 +1482,7 @@ bool ChildSession::exportSignAndUploadDocument(const char* buffer, int length, c
const Poco::Path filenameParam(filename);
const std::string aTempDocumentURL = JAILED_DOCUMENT_ROOT + aTempDir + "/" + filenameParam.getFileName();
- {
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
- getLOKitDocument()->saveAs(aTempDocumentURL.c_str(), filetype.c_str(), nullptr);
- }
+ getLOKitDocument()->saveAs(aTempDocumentURL.c_str(), filetype.c_str(), nullptr);
// sign document
{
@@ -1624,8 +1573,6 @@ bool ChildSession::exportSignAndUploadDocument(const char* buffer, int length, c
bool ChildSession::askSignatureStatus(const char* buffer, int length, const std::vector<std::string>& /*tokens*/)
{
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
bool bResult = true;
const std::string firstLine = getFirstLine(buffer, length);
@@ -1676,8 +1623,6 @@ bool ChildSession::selectGraphic(const char* /*buffer*/, int /*length*/, const s
return false;
}
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
getLOKitDocument()->setView(_viewId);
getLOKitDocument()->setGraphicSelection(type, x, y);
@@ -1693,8 +1638,6 @@ bool ChildSession::resetSelection(const char* /*buffer*/, int /*length*/, const
return false;
}
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
getLOKitDocument()->setView(_viewId);
getLOKitDocument()->resetSelection();
@@ -1743,51 +1686,48 @@ bool ChildSession::saveAs(const char* /*buffer*/, int /*length*/, const std::vec
}
bool success = false;
- {
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
- if (filterOptions.empty() && format == "html")
- {
- // Opt-in to avoid linked images, those would not leave the chroot.
- filterOptions = "EmbedImages";
- }
+ if (filterOptions.empty() && format == "html")
+ {
+ // Opt-in to avoid linked images, those would not leave the chroot.
+ filterOptions = "EmbedImages";
+ }
- // We don't have the FileId at this point, just a new filename to save-as.
- // So here the filename will be obfuscated with some hashing, which later will
- // get a proper FileId that we will use going forward.
- LOG_DBG("Calling LOK's saveAs with: '" << anonymizeUrl(wopiFilename) << "', '" <<
- (format.size() == 0 ? "(nullptr)" : format.c_str()) << "', '" <<
- (filterOptions.size() == 0 ? "(nullptr)" : filterOptions.c_str()) << "'.");
+ // We don't have the FileId at this point, just a new filename to save-as.
+ // So here the filename will be obfuscated with some hashing, which later will
+ // get a proper FileId that we will use going forward.
+ LOG_DBG("Calling LOK's saveAs with: '" << anonymizeUrl(wopiFilename) << "', '" <<
+ (format.size() == 0 ? "(nullptr)" : format.c_str()) << "', '" <<
+ (filterOptions.size() == 0 ? "(nullptr)" : filterOptions.c_str()) << "'.");
- getLOKitDocument()->setView(_viewId);
+ getLOKitDocument()->setView(_viewId);
- success = getLOKitDocument()->saveAs(url.c_str(),
- format.empty() ? nullptr : format.c_str(),
- filterOptions.empty() ? nullptr : filterOptions.c_str());
+ success = getLOKitDocument()->saveAs(url.c_str(),
+ format.empty() ? nullptr : format.c_str(),
+ filterOptions.empty() ? nullptr : filterOptions.c_str());
- if (!success)
+ if (!success)
+ {
+ // a desperate try - add an extension hoping that it'll help
+ bool retry = true;
+ switch (getLOKitDocument()->getDocumentType())
{
- // a desperate try - add an extension hoping that it'll help
- bool retry = true;
- switch (getLOKitDocument()->getDocumentType())
- {
- case LOK_DOCTYPE_TEXT: url += ".odt"; wopiFilename += ".odt"; break;
- case LOK_DOCTYPE_SPREADSHEET: url += ".ods"; wopiFilename += ".ods"; break;
- case LOK_DOCTYPE_PRESENTATION: url += ".odp"; wopiFilename += ".odp"; break;
- case LOK_DOCTYPE_DRAWING: url += ".odg"; wopiFilename += ".odg"; break;
- default: retry = false; break;
- }
+ case LOK_DOCTYPE_TEXT: url += ".odt"; wopiFilename += ".odt"; break;
+ case LOK_DOCTYPE_SPREADSHEET: url += ".ods"; wopiFilename += ".ods"; break;
+ case LOK_DOCTYPE_PRESENTATION: url += ".odp"; wopiFilename += ".odp"; break;
+ case LOK_DOCTYPE_DRAWING: url += ".odg"; wopiFilename += ".odg"; break;
+ default: retry = false; break;
+ }
- if (retry)
- {
- LOG_DBG("Retry: calling LOK's saveAs with: '" << url.c_str() << "', '" <<
- (format.size() == 0 ? "(nullptr)" : format.c_str()) << "', '" <<
- (filterOptions.size() == 0 ? "(nullptr)" : filterOptions.c_str()) << "'.");
+ if (retry)
+ {
+ LOG_DBG("Retry: calling LOK's saveAs with: '" << url.c_str() << "', '" <<
+ (format.size() == 0 ? "(nullptr)" : format.c_str()) << "', '" <<
+ (filterOptions.size() == 0 ? "(nullptr)" : filterOptions.c_str()) << "'.");
- success = getLOKitDocument()->saveAs(url.c_str(),
- format.size() == 0 ? nullptr :format.c_str(),
- filterOptions.size() == 0 ? nullptr : filterOptions.c_str());
- }
+ success = getLOKitDocument()->saveAs(url.c_str(),
+ format.size() == 0 ? nullptr :format.c_str(),
+ filterOptions.size() == 0 ? nullptr : filterOptions.c_str());
}
}
@@ -1813,8 +1753,6 @@ bool ChildSession::setClientPart(const char* /*buffer*/, int /*length*/, const s
return false;
}
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
getLOKitDocument()->setView(_viewId);
if (getLOKitDocument()->getDocumentType() != LOK_DOCTYPE_TEXT && part != getLOKitDocument()->getPart())
@@ -1835,8 +1773,6 @@ bool ChildSession::setPage(const char* /*buffer*/, int /*length*/, const std::ve
return false;
}
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
getLOKitDocument()->setView(_viewId);
getLOKitDocument()->setPart(page);
@@ -1854,8 +1790,6 @@ bool ChildSession::renderShapeSelection(const char* /*buffer*/, int /*length*/,
return false;
}
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
-
getLOKitDocument()->setView(_viewId);
char* pOutput = nullptr;
@@ -2079,7 +2013,6 @@ void ChildSession::loKitCallback(const int type, const std::string& payload)
{
//TODO: clenaup and merge.
- std::unique_lock<std::mutex> lock(_docManager.getDocumentMutex());
const int parts = getLOKitDocument()->getParts();
for (int i = 0; i < parts; ++i)
{
@@ -2091,8 +2024,6 @@ void ChildSession::loKitCallback(const int type, const std::string& payload)
" height=" + std::to_string(INT_MAX));
}
- lock.unlock();
-
getStatus("", 0);
}
break;
diff --git a/kit/ChildSession.hpp b/kit/ChildSession.hpp
index 597002336..215d1ad4b 100644
--- a/kit/ChildSession.hpp
+++ b/kit/ChildSession.hpp
@@ -68,10 +68,6 @@ public:
virtual std::map<int, UserInfo> getViewInfo() = 0;
virtual std::mutex& getMutex() = 0;
- /// Mutex guarding the document - so that we can lock operations like
- /// setting a view followed by a tile render, etc.
- virtual std::mutex& getDocumentMutex() = 0;
-
virtual std::string getObfuscatedFileId() = 0;
virtual std::shared_ptr<TileQueue>& getTileQueue() = 0;
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index b0f3e32c2..2f0d63f21 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -1021,8 +1021,6 @@ public:
" passwordProvided=" << _haveDocPassword <<
" password='" << _docPassword << "'");
- Util::assertIsLocked(_documentMutex);
-
if (_isDocPasswordProtected && _haveDocPassword)
{
// it means this is the second attempt with the wrong password; abort the load operation
@@ -1098,7 +1096,6 @@ public:
const size_t pixmapSize = 4 * pixmapWidth * pixmapHeight;
std::vector<unsigned char> pixmap(pixmapSize, 0);
- std::unique_lock<std::mutex> lock(_documentMutex);
if (!_loKitDocument)
{
LOG_ERR("Tile rendering requested before loading document.");
@@ -1516,7 +1513,6 @@ private:
const int viewId = session.getViewId();
_tileQueue->removeCursorPosition(viewId);
- std::unique_lock<std::mutex> lockLokDoc(_documentMutex);
if (_loKitDocument == nullptr)
{
LOG_ERR("Unloading session [" << sessionId << "] without loKitDocument.");
@@ -1591,8 +1587,6 @@ private:
/// Notify all views of viewId and their associated usernames
void notifyViewInfo() override
{
- Util::assertIsLocked(_documentMutex);
-
// Get the list of view ids from the core
const int viewCount = getLOKitDocument()->getViewsCount();
std::vector<int> viewIds(viewCount);
@@ -1693,8 +1687,6 @@ private:
// Get the color value for all author names from the core
std::map<std::string, int> getViewColors()
{
- Util::assertIsLocked(_documentMutex);
-
char* values = _loKitDocument->getCommandValues(".uno:TrackedChangeAuthors");
const std::string colorValues = std::string(values == nullptr ? "" : values);
std::free(values);
@@ -1745,8 +1737,6 @@ private:
if (!lang.empty())
options = "Language=" + lang;
- std::unique_lock<std::mutex> lock(_documentMutex);
-
if (!_loKitDocument)
{
// This is the first time we are loading the document
@@ -2133,12 +2123,6 @@ private:
return _loKitDocument;
}
- /// Return access to the lok::Document instance.
- std::mutex& getDocumentMutex() override
- {
- return _documentMutex;
- }
-
std::string getObfuscatedFileId() override
{
return _obfuscatedFileId;
@@ -2178,10 +2162,6 @@ private:
std::atomic<bool> _stop;
mutable std::mutex _mutex;
- /// Mutex guarding the lok::Document so that we can lock operations
- /// like setting a view followed by a tile render, etc.
- std::mutex _documentMutex;
-
ThreadPool _pngPool;
std::condition_variable _cvLoading;
diff --git a/test/WhiteBoxTests.cpp b/test/WhiteBoxTests.cpp
index 21ca04bd1..38bb627d0 100644
--- a/test/WhiteBoxTests.cpp
+++ b/test/WhiteBoxTests.cpp
@@ -542,11 +542,6 @@ public:
return _mutex;
}
- std::mutex& getDocumentMutex() override
- {
- return _mutex;
- }
-
std::string getObfuscatedFileId() override
{
return std::string();