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

[Ashod Nakashian]

 common/MessageQueue.hpp |   18 ++++++++++++++----
 kit/Kit.cpp             |    4 ----
 2 files changed, 14 insertions(+), 8 deletions(-)

New commits:
commit 1a6098b92500864b6bda525625ed6372caae8607
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun Jan 1 13:04:24 2017 -0500

    wsd: TileQueue should be fully threadsafe
    
    There should be no need to take a lock to
    access it. However there were cases where it
    wasn't thread-safe. Now we can remove unncessary
    locking before invoking it.
    
    Change-Id: I90d2c6940610a59aa6c749491ea85fb80b0acbcd
    Reviewed-on: https://gerrit.libreoffice.org/32615
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/common/MessageQueue.hpp b/common/MessageQueue.hpp
index afea17f..8bbe56a 100644
--- a/common/MessageQueue.hpp
+++ b/common/MessageQueue.hpp
@@ -51,10 +51,6 @@ public:
     /// Thread safe remove_if.
     void remove_if(const std::function<bool(const Payload&)>& pred);
 
-private:
-    std::mutex _mutex;
-    std::condition_variable _cv;
-
 protected:
     virtual void put_impl(const Payload& value);
 
@@ -64,7 +60,16 @@ protected:
 
     void clear_impl();
 
+    /// Get the queue lock when accessing members of derived classes.
+    std::unique_lock<std::mutex> getLock() { return std::unique_lock<std::mutex>(_mutex); }
+
+protected:
     std::vector<Payload> _queue;
+
+private:
+    std::mutex _mutex;
+    std::condition_variable _cv;
+
 };
 
 /** MessageQueue specialized for priority handling of tiles.
@@ -96,6 +101,9 @@ public:
     void updateCursorPosition(int viewId, int part, int x, int y, int width, int height)
     {
         auto cursorPosition = CursorPosition({ part, x, y, width, height });
+
+        auto lock = getLock();
+
         auto it = _cursorPositions.find(viewId);
         if (it != _cursorPositions.end())
         {
@@ -119,6 +127,8 @@ public:
 
     void removeCursorPosition(int viewId)
     {
+        auto lock = getLock();
+
         const auto view = std::find(_viewOrder.begin(), _viewOrder.end(), viewId);
         if (view != _viewOrder.end())
         {
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index ecc8dc7..286760d 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -797,8 +797,6 @@ public:
                 "] [" << LOKitHelper::kitCallbackTypeToString(nType) <<
                 "] [" << payload << "].");
 
-        std::unique_lock<std::mutex> lock(pDescr->Doc->getMutex());
-
         if (nType == LOK_CALLBACK_INVALIDATE_VISIBLE_CURSOR ||
             nType == LOK_CALLBACK_CELL_CURSOR)
         {
@@ -844,8 +842,6 @@ private:
     /// Helper method to broadcast callback and its payload to all clients
     void broadcastCallbackToClients(const int nType, const std::string& payload)
     {
-        std::unique_lock<std::mutex> lock(_mutex);
-
         // "-1" means broadcast
         _tileQueue->put("callback -1 " + std::to_string(nType) + " " + payload);
     }