[Libreoffice-commits] online.git: kit/Kit.cpp

[Ashod Nakashian]

 kit/Kit.cpp |   18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

New commits:
commit 02b69738cae872aba095113831915daa999471a5
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Fri Dec 23 17:33:37 2016 -0500

    wsd: always lock documentMutex around loKitDocument
    
    The _documentMutex used to be a member of _loKitDocument,
    so it couldn't be locked when _loKitDocument was null.
    
    However this has long since changed and is independent.
    So no good reason not to lock it at every access point
    of _loKitDocument, which is simpler and safer.
    
    Change-Id: I82bdff5f94b5eaf68365d8cdb3ef5779e6c6cc87
    Reviewed-on: https://gerrit.libreoffice.org/32606
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index 5dcd695..4a1ae08 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -612,13 +612,13 @@ public:
         std::vector<unsigned char> pixmap;
         pixmap.resize(output.capacity());
 
+        std::unique_lock<std::mutex> lock(_documentMutex);
         if (!_loKitDocument)
         {
             LOG_ERR("Tile rendering requested before loading document.");
             return;
         }
 
-        std::unique_lock<std::mutex> lock(_documentMutex);
         if (_loKitDocument->getViewsCount() <= 0)
         {
             LOG_ERR("Tile rendering requested without views.");
@@ -685,13 +685,13 @@ public:
         const size_t pixmapSize = 4 * area;
         std::vector<unsigned char> pixmap(pixmapSize, 0);
 
+        std::unique_lock<std::mutex> lock(_documentMutex);
         if (!_loKitDocument)
         {
             LOG_ERR("Tile rendering requested before loading document.");
             return;
         }
 
-        std::unique_lock<std::mutex> lock(_documentMutex);
         if (_loKitDocument->getViewsCount() <= 0)
         {
             LOG_ERR("Tile rendering requested without views.");
@@ -934,14 +934,14 @@ private:
         const auto viewId = session.getViewId();
         _tileQueue->removeCursorPosition(viewId);
 
+
+        std::unique_lock<std::mutex> lockLokDoc(_documentMutex);
         if (_loKitDocument == nullptr)
         {
             LOG_ERR("Unloading session [" << sessionId << "] without loKitDocument.");
             return;
         }
 
-        std::unique_lock<std::mutex> lockLokDoc(_documentMutex);
-
         _loKitDocument->setView(viewId);
         _loKitDocument->registerCallback(nullptr, nullptr);
         _loKitDocument->destroyView(viewId);
@@ -1098,7 +1098,7 @@ private:
     {
         const std::string sessionId = session->getId();
 
-        std::unique_lock<std::mutex> lockLokDoc;
+        std::unique_lock<std::mutex> lock(_documentMutex);
 
         if (!_loKitDocument)
         {
@@ -1123,9 +1123,6 @@ private:
             _loKitDocument.reset(_loKit->documentLoad(uri.c_str()));
             LOG_DBG("Returned lokit::documentLoad(" << uri << ") in " << (timestamp.elapsed()  / 1000.) << "ms.");
 
-            std::unique_lock<std::mutex> l(_documentMutex);
-            lockLokDoc.swap(l);
-
             if (!_loKitDocument || !_loKitDocument->get())
             {
                 LOG_ERR("Failed to load: " << uri << ", error: " << _loKit->getError());
@@ -1160,9 +1157,6 @@ private:
         }
         else
         {
-            std::unique_lock<std::mutex> l(_documentMutex);
-            lockLokDoc.swap(l);
-
             LOG_INF("Document with url [" << uri << "] already loaded. Need to create new view for session [" << sessionId << "].");
 
             // Check if this document requires password
@@ -1190,8 +1184,6 @@ private:
             LOG_TRC("View to url [" << uri << "] created.");
         }
 
-        Util::assertIsLocked(lockLokDoc);
-
         Object::Ptr renderOptsObj;
 
         // Fill the object with renderoptions, if any