[Libreoffice-commits] online.git: loolwsd/ChildSession.cpp loolwsd/ChildSession.hpp loolwsd/LOOLKit.cpp

Ashod Nakashian ashod.nakashian at collabora.co.uk
Mon Aug 15 03:10:15 UTC 2016


 loolwsd/ChildSession.cpp |  437 ++++++++++++++++++-----------------------------
 loolwsd/ChildSession.hpp |    6 
 loolwsd/LOOLKit.cpp      |  118 ++++++++++--
 3 files changed, 273 insertions(+), 288 deletions(-)

New commits:
commit e3507552b7a342b961f9a0d786294da4e13b80a4
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sat Aug 13 18:25:06 2016 -0400

    loolwsd: one thread and queue per document
    
    Each ChildSession had its own thread and queue,
    which was an overkill. By moving the LOK callback
    handler into Document and handling all events in the
    same order that we receive them we reduce resource
    consumption without affecting performance.
    
    In fact, performance could improve by avoiding
    unnecessary overheads.
    
    Change-Id: Ic2bac0f08c28e91acabd512a704966c6b761fc7c
    Reviewed-on: https://gerrit.libreoffice.org/28124
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/ChildSession.cpp b/loolwsd/ChildSession.cpp
index 9a89b66..aedcfa6 100644
--- a/loolwsd/ChildSession.cpp
+++ b/loolwsd/ChildSession.cpp
@@ -46,258 +46,6 @@ using Poco::StringTokenizer;
 using Poco::Timestamp;
 using Poco::URI;
 
-class CallbackNotification: public Notification
-{
-public:
-    typedef AutoPtr<CallbackNotification> Ptr;
-
-    CallbackNotification(const int nType, const std::string& rPayload)
-      : _nType(nType),
-        _aPayload(rPayload)
-    {
-    }
-
-    const int _nType;
-    const std::string _aPayload;
-};
-
-/// This thread handles callbacks from the lokit instance.
-class CallbackWorker: public Runnable
-{
-public:
-    CallbackWorker(NotificationQueue& queue, ChildSession& session):
-        _queue(queue),
-        _session(session),
-        _stop(false)
-    {
-    }
-
-    void callback(const int nType, const std::string& rPayload)
-    {
-        auto lock = _session.getLock();
-
-        // Cache important notifications to replay them when our client
-        // goes inactive and loses them.
-        if (nType == LOK_CALLBACK_INVALIDATE_VISIBLE_CURSOR ||
-            nType == LOK_CALLBACK_CURSOR_VISIBLE ||
-            nType == LOK_CALLBACK_CELL_CURSOR ||
-            nType == LOK_CALLBACK_CELL_FORMULA ||
-            nType == LOK_CALLBACK_GRAPHIC_SELECTION ||
-            nType == LOK_CALLBACK_TEXT_SELECTION ||
-            nType == LOK_CALLBACK_TEXT_SELECTION_START ||
-            nType == LOK_CALLBACK_TEXT_SELECTION_END ||
-            nType == LOK_CALLBACK_DOCUMENT_SIZE_CHANGED)
-        {
-            _session.setDocState(nType, rPayload);
-        }
-
-        const auto typeName = LOKitHelper::kitCallbackTypeToString(nType);
-        if (_session.isCloseFrame())
-        {
-            Log::trace("Skipping callback [" + typeName + "] on closing session " + _session.getName());
-            return;
-        }
-        else if (_session.isDisconnected())
-        {
-            Log::trace("Skipping callback [" + typeName + "] on disconnected session " + _session.getName());
-            return;
-        }
-        else if (!_session.isActive())
-        {
-            // Pass save notifications through.
-            if (nType != LOK_CALLBACK_UNO_COMMAND_RESULT || rPayload.find(".uno:Save") == std::string::npos)
-            {
-                Log::trace("Skipping callback [" + typeName + "] on inactive session " + _session.getName());
-                return;
-            }
-        }
-
-        Log::trace() << "CallbackWorker::callback [" << _session.getName() << "]: "
-                     << typeName << " [" << rPayload << "]." << Log::end;
-        switch (nType)
-        {
-        case LOK_CALLBACK_INVALIDATE_TILES:
-            {
-                const auto curPart = _session.getPart();
-
-                StringTokenizer tokens(rPayload, " ", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
-                if (tokens.count() == 4)
-                {
-                    int x, y, width, height;
-                    try
-                    {
-                        x = std::stoi(tokens[0]);
-                        y = std::stoi(tokens[1]);
-                        width = std::stoi(tokens[2]);
-                        height = std::stoi(tokens[3]);
-                    }
-                    catch (const std::out_of_range&)
-                    {
-                        // something went wrong, invalidate everything
-                        Log::warn("Ignoring integer values out of range: " + rPayload);
-                        x = 0;
-                        y = 0;
-                        width = INT_MAX;
-                        height = INT_MAX;
-                    }
-
-                    _session.sendTextFrame("invalidatetiles:"
-                                           " part=" + std::to_string(curPart) +
-                                           " x=" + std::to_string(x) +
-                                           " y=" + std::to_string(y) +
-                                           " width=" + std::to_string(width) +
-                                           " height=" + std::to_string(height));
-                }
-                else
-                {
-                    _session.sendTextFrame("invalidatetiles: " + rPayload);
-                }
-            }
-            break;
-        case LOK_CALLBACK_INVALIDATE_VISIBLE_CURSOR:
-            _session.sendTextFrame("invalidatecursor: " + rPayload);
-            break;
-        case LOK_CALLBACK_TEXT_SELECTION:
-            _session.sendTextFrame("textselection: " + rPayload);
-            break;
-        case LOK_CALLBACK_TEXT_SELECTION_START:
-            _session.sendTextFrame("textselectionstart: " + rPayload);
-            break;
-        case LOK_CALLBACK_TEXT_SELECTION_END:
-            _session.sendTextFrame("textselectionend: " + rPayload);
-            break;
-        case LOK_CALLBACK_CURSOR_VISIBLE:
-            _session.sendTextFrame("cursorvisible: " + rPayload);
-            break;
-        case LOK_CALLBACK_GRAPHIC_SELECTION:
-            _session.sendTextFrame("graphicselection: " + rPayload);
-            break;
-        case LOK_CALLBACK_CELL_CURSOR:
-            _session.sendTextFrame("cellcursor: " + rPayload);
-            break;
-        case LOK_CALLBACK_CELL_FORMULA:
-            _session.sendTextFrame("cellformula: " + rPayload);
-            break;
-        case LOK_CALLBACK_MOUSE_POINTER:
-            _session.sendTextFrame("mousepointer: " + rPayload);
-            break;
-        case LOK_CALLBACK_HYPERLINK_CLICKED:
-            _session.sendTextFrame("hyperlinkclicked: " + rPayload);
-            break;
-        case LOK_CALLBACK_STATE_CHANGED:
-            _session.sendTextFrame("statechanged: " + rPayload);
-            break;
-        case LOK_CALLBACK_SEARCH_NOT_FOUND:
-            _session.sendTextFrame("searchnotfound: " + rPayload);
-            break;
-        case LOK_CALLBACK_SEARCH_RESULT_SELECTION:
-            _session.sendTextFrame("searchresultselection: " + rPayload);
-            break;
-        case LOK_CALLBACK_DOCUMENT_SIZE_CHANGED:
-            _session.getStatus("", 0);
-            _session.getPartPageRectangles("", 0);
-            break;
-        case LOK_CALLBACK_SET_PART:
-            _session.sendTextFrame("setpart: " + rPayload);
-            break;
-        case LOK_CALLBACK_UNO_COMMAND_RESULT:
-            _session.sendTextFrame("unocommandresult: " + rPayload);
-            break;
-        case LOK_CALLBACK_ERROR:
-            {
-                Parser parser;
-                Poco::Dynamic::Var var = parser.parse(rPayload);
-                Object::Ptr object = var.extract<Object::Ptr>();
-
-                _session.sendTextFrame("error: cmd=" + object->get("cmd").toString() +
-                        " kind=" + object->get("kind").toString() + " code=" + object->get("code").toString());
-            }
-            break;
-        case LOK_CALLBACK_CONTEXT_MENU:
-            _session.sendTextFrame("contextmenu: " + rPayload);
-            break;
-        case LOK_CALLBACK_STATUS_INDICATOR_START:
-            _session.sendTextFrame("statusindicatorstart:");
-            break;
-        case LOK_CALLBACK_STATUS_INDICATOR_SET_VALUE:
-            _session.sendTextFrame("statusindicatorsetvalue: " + rPayload);
-            break;
-        case LOK_CALLBACK_STATUS_INDICATOR_FINISH:
-            _session.sendTextFrame("statusindicatorfinish:");
-            break;
-        case LOK_CALLBACK_INVALIDATE_VIEW_CURSOR:
-            _session.sendTextFrame("invalidateviewcursor: " + rPayload);
-            break;
-        case LOK_CALLBACK_TEXT_VIEW_SELECTION:
-            _session.sendTextFrame("textviewselection: " + rPayload);
-            break;
-        case LOK_CALLBACK_CELL_VIEW_CURSOR:
-            _session.sendTextFrame("cellviewcursor: " + rPayload);
-            break;
-        case LOK_CALLBACK_GRAPHIC_VIEW_SELECTION:
-            _session.sendTextFrame("graphicviewselection: " + rPayload);
-            break;
-        case LOK_CALLBACK_VIEW_CURSOR_VISIBLE:
-            _session.sendTextFrame("viewcursorvisible: " + rPayload);
-            break;
-        case LOK_CALLBACK_VIEW_LOCK:
-            _session.sendTextFrame("viewlock: " + rPayload);
-            break;
-        default:
-            Log::error("Unknown callback event (" + std::to_string(nType) + "): " + rPayload);
-        }
-    }
-
-    void run()
-    {
-        Util::setThreadName("kit_callback");
-
-        Log::debug("Thread started.");
-
-        while (!_stop && !TerminationFlag)
-        {
-            Notification::Ptr aNotification(_queue.waitDequeueNotification());
-            if (!_stop && !TerminationFlag && aNotification)
-            {
-                CallbackNotification::Ptr aCallbackNotification = aNotification.cast<CallbackNotification>();
-                assert(aCallbackNotification);
-
-                const auto nType = aCallbackNotification->_nType;
-                try
-                {
-                    callback(nType, aCallbackNotification->_aPayload);
-                }
-                catch (const Exception& exc)
-                {
-                    Log::error() << "CallbackWorker::run: Exception while handling callback [" << LOKitHelper::kitCallbackTypeToString(nType) << "]: "
-                                 << exc.displayText()
-                                 << (exc.nested() ? " (" + exc.nested()->displayText() + ")" : "")
-                                 << Log::end;
-                }
-                catch (const std::exception& exc)
-                {
-                    Log::error("CallbackWorker::run: Exception while handling callback [" + LOKitHelper::kitCallbackTypeToString(nType) + "]: " + exc.what());
-                }
-            }
-            else
-                break;
-        }
-
-        Log::debug("Thread finished.");
-    }
-
-    void stop()
-    {
-        _stop = true;
-        _queue.wakeUpAll();
-    }
-
-private:
-    NotificationQueue& _queue;
-    ChildSession& _session;
-    volatile bool _stop;
-};
-
 std::recursive_mutex ChildSession::Mutex;
 
 ChildSession::ChildSession(const std::string& id,
@@ -310,12 +58,9 @@ ChildSession::ChildSession(const std::string& id,
     _jailId(jailId),
     _viewId(0),
     _onLoad(std::move(onLoad)),
-    _onUnload(std::move(onUnload)),
-    _callbackWorker(new CallbackWorker(_callbackQueue, *this))
+    _onUnload(std::move(onUnload))
 {
     Log::info("ChildSession ctor [" + getName() + "].");
-
-    _callbackThread.start(*_callbackWorker);
 }
 
 ChildSession::~ChildSession()
@@ -323,10 +68,6 @@ ChildSession::~ChildSession()
     Log::info("~ChildSession dtor [" + getName() + "].");
 
     disconnect();
-
-    // Wait for the callback worker to finish.
-    _callbackWorker->stop();
-    _callbackThread.join();
 }
 
 void ChildSession::disconnect()
@@ -1173,10 +914,180 @@ bool ChildSession::setPage(const char* /*buffer*/, int /*length*/, StringTokeniz
     return true;
 }
 
-void ChildSession::loKitCallback(const int nType, const std::string& payload)
+void ChildSession::loKitCallback(const int nType, const std::string& rPayload)
 {
-    auto pNotif = new CallbackNotification(nType, payload);
-    _callbackQueue.enqueueNotification(pNotif);
+    std::unique_lock<std::recursive_mutex> lock(Mutex);
+
+    // Cache important notifications to replay them when our client
+    // goes inactive and loses them.
+    if (nType == LOK_CALLBACK_INVALIDATE_VISIBLE_CURSOR ||
+        nType == LOK_CALLBACK_CURSOR_VISIBLE ||
+        nType == LOK_CALLBACK_CELL_CURSOR ||
+        nType == LOK_CALLBACK_CELL_FORMULA ||
+        nType == LOK_CALLBACK_GRAPHIC_SELECTION ||
+        nType == LOK_CALLBACK_TEXT_SELECTION ||
+        nType == LOK_CALLBACK_TEXT_SELECTION_START ||
+        nType == LOK_CALLBACK_TEXT_SELECTION_END ||
+        nType == LOK_CALLBACK_DOCUMENT_SIZE_CHANGED)
+    {
+        setDocState(nType, rPayload);
+    }
+
+    const auto typeName = LOKitHelper::kitCallbackTypeToString(nType);
+    if (isCloseFrame())
+    {
+        Log::trace("Skipping callback [" + typeName + "] on closing session " + getName());
+        return;
+    }
+    else if (isDisconnected())
+    {
+        Log::trace("Skipping callback [" + typeName + "] on disconnected session " + getName());
+        return;
+    }
+    else if (!isActive())
+    {
+        // Pass save notifications through.
+        if (nType != LOK_CALLBACK_UNO_COMMAND_RESULT || rPayload.find(".uno:Save") == std::string::npos)
+        {
+            Log::trace("Skipping callback [" + typeName + "] on inactive session " + getName());
+            return;
+        }
+    }
+
+    Log::trace() << "CallbackWorker::callback [" << getName() << "]: "
+                 << typeName << " [" << rPayload << "]." << Log::end;
+    switch (nType)
+    {
+    case LOK_CALLBACK_INVALIDATE_TILES:
+        {
+            const auto curPart = getPart();
+
+            StringTokenizer tokens(rPayload, " ", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
+            if (tokens.count() == 4)
+            {
+                int x, y, width, height;
+                try
+                {
+                    x = std::stoi(tokens[0]);
+                    y = std::stoi(tokens[1]);
+                    width = std::stoi(tokens[2]);
+                    height = std::stoi(tokens[3]);
+                }
+                catch (const std::out_of_range&)
+                {
+                    // something went wrong, invalidate everything
+                    Log::warn("Ignoring integer values out of range: " + rPayload);
+                    x = 0;
+                    y = 0;
+                    width = INT_MAX;
+                    height = INT_MAX;
+                }
+
+                sendTextFrame("invalidatetiles:"
+                                       " part=" + std::to_string(curPart) +
+                                       " x=" + std::to_string(x) +
+                                       " y=" + std::to_string(y) +
+                                       " width=" + std::to_string(width) +
+                                       " height=" + std::to_string(height));
+            }
+            else
+            {
+                sendTextFrame("invalidatetiles: " + rPayload);
+            }
+        }
+        break;
+    case LOK_CALLBACK_INVALIDATE_VISIBLE_CURSOR:
+        sendTextFrame("invalidatecursor: " + rPayload);
+        break;
+    case LOK_CALLBACK_TEXT_SELECTION:
+        sendTextFrame("textselection: " + rPayload);
+        break;
+    case LOK_CALLBACK_TEXT_SELECTION_START:
+        sendTextFrame("textselectionstart: " + rPayload);
+        break;
+    case LOK_CALLBACK_TEXT_SELECTION_END:
+        sendTextFrame("textselectionend: " + rPayload);
+        break;
+    case LOK_CALLBACK_CURSOR_VISIBLE:
+        sendTextFrame("cursorvisible: " + rPayload);
+        break;
+    case LOK_CALLBACK_GRAPHIC_SELECTION:
+        sendTextFrame("graphicselection: " + rPayload);
+        break;
+    case LOK_CALLBACK_CELL_CURSOR:
+        sendTextFrame("cellcursor: " + rPayload);
+        break;
+    case LOK_CALLBACK_CELL_FORMULA:
+        sendTextFrame("cellformula: " + rPayload);
+        break;
+    case LOK_CALLBACK_MOUSE_POINTER:
+        sendTextFrame("mousepointer: " + rPayload);
+        break;
+    case LOK_CALLBACK_HYPERLINK_CLICKED:
+        sendTextFrame("hyperlinkclicked: " + rPayload);
+        break;
+    case LOK_CALLBACK_STATE_CHANGED:
+        sendTextFrame("statechanged: " + rPayload);
+        break;
+    case LOK_CALLBACK_SEARCH_NOT_FOUND:
+        sendTextFrame("searchnotfound: " + rPayload);
+        break;
+    case LOK_CALLBACK_SEARCH_RESULT_SELECTION:
+        sendTextFrame("searchresultselection: " + rPayload);
+        break;
+    case LOK_CALLBACK_DOCUMENT_SIZE_CHANGED:
+        getStatus("", 0);
+        getPartPageRectangles("", 0);
+        break;
+    case LOK_CALLBACK_SET_PART:
+        sendTextFrame("setpart: " + rPayload);
+        break;
+    case LOK_CALLBACK_UNO_COMMAND_RESULT:
+        sendTextFrame("unocommandresult: " + rPayload);
+        break;
+    case LOK_CALLBACK_ERROR:
+        {
+            Parser parser;
+            Poco::Dynamic::Var var = parser.parse(rPayload);
+            Object::Ptr object = var.extract<Object::Ptr>();
+
+            sendTextFrame("error: cmd=" + object->get("cmd").toString() +
+                    " kind=" + object->get("kind").toString() + " code=" + object->get("code").toString());
+        }
+        break;
+    case LOK_CALLBACK_CONTEXT_MENU:
+        sendTextFrame("contextmenu: " + rPayload);
+        break;
+    case LOK_CALLBACK_STATUS_INDICATOR_START:
+        sendTextFrame("statusindicatorstart:");
+        break;
+    case LOK_CALLBACK_STATUS_INDICATOR_SET_VALUE:
+        sendTextFrame("statusindicatorsetvalue: " + rPayload);
+        break;
+    case LOK_CALLBACK_STATUS_INDICATOR_FINISH:
+        sendTextFrame("statusindicatorfinish:");
+        break;
+    case LOK_CALLBACK_INVALIDATE_VIEW_CURSOR:
+        sendTextFrame("invalidateviewcursor: " + rPayload);
+        break;
+    case LOK_CALLBACK_TEXT_VIEW_SELECTION:
+        sendTextFrame("textviewselection: " + rPayload);
+        break;
+    case LOK_CALLBACK_CELL_VIEW_CURSOR:
+        sendTextFrame("cellviewcursor: " + rPayload);
+        break;
+    case LOK_CALLBACK_GRAPHIC_VIEW_SELECTION:
+        sendTextFrame("graphicviewselection: " + rPayload);
+        break;
+    case LOK_CALLBACK_VIEW_CURSOR_VISIBLE:
+        sendTextFrame("viewcursorvisible: " + rPayload);
+        break;
+    case LOK_CALLBACK_VIEW_LOCK:
+        sendTextFrame("viewlock: " + rPayload);
+        break;
+    default:
+        Log::error("Unknown callback event (" + std::to_string(nType) + "): " + rPayload);
+    }
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/ChildSession.hpp b/loolwsd/ChildSession.hpp
index 1481a9f..5995397 100644
--- a/loolwsd/ChildSession.hpp
+++ b/loolwsd/ChildSession.hpp
@@ -47,7 +47,7 @@ public:
 
     void setDocState(const int type, const std::string& payload) { _lastDocStates[type] = payload; }
 
-    void loKitCallback(const int nType, const std::string& payload);
+    void loKitCallback(const int nType, const std::string& rPayload);
 
     static std::unique_lock<std::recursive_mutex> getLock() { return std::unique_lock<std::recursive_mutex>(Mutex); }
 
@@ -88,10 +88,6 @@ private:
     OnLoadCallback _onLoad;
     OnUnloadCallback _onUnload;
 
-    std::unique_ptr<CallbackWorker> _callbackWorker;
-    Poco::Thread _callbackThread;
-    Poco::NotificationQueue _callbackQueue;
-
     /// Synchronize _loKitDocument acess.
     /// This should be owned by Document.
     static std::recursive_mutex Mutex;
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 74d259c..bb67ef9 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -26,7 +26,6 @@
 #include <cstdlib>
 #include <iostream>
 #include <memory>
-#include <regex>
 
 #define LOK_USE_UNSTABLE_API
 #include <LibreOfficeKit/LibreOfficeKitInit.h>
@@ -37,6 +36,7 @@
 #include <Poco/Net/HTTPResponse.h>
 #include <Poco/Net/NetException.h>
 #include <Poco/Net/WebSocket.h>
+#include <Poco/NotificationQueue.h>
 #include <Poco/Process.h>
 #include <Poco/Runnable.h>
 #include <Poco/StringTokenizer.h>
@@ -66,21 +66,24 @@ typedef int (LokHookPreInit)  (const char *install_path, const char *user_profil
 
 using namespace LOOLProtocol;
 
+using Poco::AutoPtr;
 using Poco::Exception;
 using Poco::File;
-using Poco::Net::NetException;
 using Poco::Net::HTTPClientSession;
-using Poco::Net::HTTPResponse;
 using Poco::Net::HTTPRequest;
+using Poco::Net::HTTPResponse;
+using Poco::Net::NetException;
 using Poco::Net::WebSocket;
+using Poco::Notification;
+using Poco::NotificationQueue;
 using Poco::Path;
 using Poco::Process;
 using Poco::Runnable;
 using Poco::StringTokenizer;
 using Poco::Thread;
 using Poco::Timestamp;
-using Poco::Util::Application;
 using Poco::URI;
+using Poco::Util::Application;
 
 namespace
 {
@@ -339,15 +342,24 @@ private:
     std::atomic<bool> _joined;
 };
 
-/// Regex to parse the ViewId from json.
-static std::regex ViewIdRegex("\"viewId\"\\s*:\\s*\"(\\d*)\"");
+/// Worker callback notification object.
+/// Used to pass callback data to the worker
+/// thread to invoke sessions with the data.
+class CallbackNotification : public Notification
+{
+public:
+    typedef AutoPtr<CallbackNotification> Ptr;
 
-class Document;
+    CallbackNotification(const std::shared_ptr<ChildSession>& session, const int nType, const std::string& rPayload)
+      : _session(session),
+        _nType(nType),
+        _aPayload(rPayload)
+    {
+    }
 
-struct CallbackDescriptor
-{
-    const Document* const Doc;
-    const unsigned ViewId;
+    const std::shared_ptr<ChildSession> _session;
+    const int _nType;
+    const std::string _aPayload;
 };
 
 /// A document container.
@@ -357,7 +369,7 @@ struct CallbackDescriptor
 /// per process. But for security reasons don't.
 /// However, we could have a loolkit instance
 /// per user or group of users (a trusted circle).
-class Document
+class Document : public Runnable
 {
 public:
     /// We have two types of password protected documents
@@ -365,6 +377,15 @@ public:
     /// 2) Document which require password to modify
     enum class PasswordType { ToView, ToModify };
 
+    /// Descriptor class used to link a LOK
+    /// callback to a specific view.
+    struct CallbackDescriptor
+    {
+        Document* const Doc;
+        const unsigned ViewId;
+    };
+
+public:
     Document(const std::shared_ptr<lok::Office>& loKit,
              const std::string& jailId,
              const std::string& docKey,
@@ -378,12 +399,15 @@ public:
         _haveDocPassword(false),
         _isDocPasswordProtected(false),
         _docPasswordType(PasswordType::ToView),
+        _stop(false),
         _isLoading(0),
         _clientViews(0)
     {
         Log::info("Document ctor for url [" + _url + "] on child [" + _jailId +
                   "] LOK_VIEW_CALLBACK=" + std::to_string(_multiView) + ".");
         assert(_loKit && _loKit->get());
+
+        _callbackThread.start(*this);
     }
 
     ~Document()
@@ -391,6 +415,10 @@ public:
         Log::info("~Document dtor for url [" + _url + "] on child [" + _jailId +
                   "]. There are " + std::to_string(_clientViews) + " views.");
 
+        // Wait for the callback worker to finish.
+        stop();
+        _callbackThread.join();
+
         // Flag all connections to stop.
         for (auto aIterator : _connections)
         {
@@ -735,22 +763,24 @@ private:
         Log::trace() << "Document::ViewCallback "
                      << LOKitHelper::kitCallbackTypeToString(nType)
                      << " [" << payload << "]." << Log::end;
+
         CallbackDescriptor* pDescr = reinterpret_cast<CallbackDescriptor*>(pData);
         assert(pDescr && "Null callback data.");
         assert(pDescr->Doc && "Null Document instance.");
 
-        std::unique_lock<std::mutex> lock(pDescr->Doc->_mutex);
-
         // Forward to the same view only.
         // Demultiplexing is done by Core.
         // TODO: replace with a map to be faster.
-        for (auto& it: pDescr->Doc->_connections)
+        for (auto& it : pDescr->Doc->_connections)
         {
-            auto session = it.second->getSession();
-            if (session && it.second->isRunning() &&
-                session->getViewId() == pDescr->ViewId)
+            if (it.second->isRunning())
             {
-                session->loKitCallback(nType, payload);
+                auto session = it.second->getSession();
+                if (session && session->getViewId() == pDescr->ViewId)
+                {
+                    auto pNotif = new CallbackNotification(session, nType, payload);
+                    pDescr->Doc->_callbackQueue.enqueueNotification(pNotif);
+                }
             }
         }
     }
@@ -785,7 +815,8 @@ private:
                 auto session = it.second->getSession();
                 if (session)
                 {
-                    session->loKitCallback(nType, payload);
+                    auto pNotif = new CallbackNotification(session, nType, payload);
+                    self->_callbackQueue.enqueueNotification(pNotif);
                 }
             }
         }
@@ -995,6 +1026,50 @@ private:
         return _loKitDocument;
     }
 
+    void run()
+    {
+        Util::setThreadName("kit_callback");
+
+        Log::debug("Thread started.");
+
+        while (!_stop && !TerminationFlag)
+        {
+            Notification::Ptr aNotification(_callbackQueue.waitDequeueNotification());
+            if (!_stop && !TerminationFlag && aNotification)
+            {
+                CallbackNotification::Ptr aCallbackNotification = aNotification.cast<CallbackNotification>();
+                assert(aCallbackNotification);
+
+                const auto nType = aCallbackNotification->_nType;
+                try
+                {
+                    aCallbackNotification->_session->loKitCallback(nType, aCallbackNotification->_aPayload);
+                }
+                catch (const Exception& exc)
+                {
+                    Log::error() << "CallbackWorker::run: Exception while handling callback [" << LOKitHelper::kitCallbackTypeToString(nType) << "]: "
+                                 << exc.displayText()
+                                 << (exc.nested() ? " (" + exc.nested()->displayText() + ")" : "")
+                                 << Log::end;
+                }
+                catch (const std::exception& exc)
+                {
+                    Log::error("CallbackWorker::run: Exception while handling callback [" + LOKitHelper::kitCallbackTypeToString(nType) + "]: " + exc.what());
+                }
+            }
+            else
+                break;
+        }
+
+        Log::debug("Thread finished.");
+    }
+
+    void stop()
+    {
+        _stop = true;
+        _callbackQueue.wakeUpAll();
+    }
+
 private:
 
     const bool _multiView;
@@ -1016,11 +1091,14 @@ private:
     // Whether password is required to view the document, or modify it
     PasswordType _docPasswordType;
 
+    std::atomic<bool> _stop;
     mutable std::mutex _mutex;
     std::condition_variable _cvLoading;
     std::atomic_size_t _isLoading;
     std::map<unsigned, std::unique_ptr<CallbackDescriptor>> _viewIdToCallbackDescr;
     std::map<unsigned, std::shared_ptr<Connection>> _connections;
+    Poco::Thread _callbackThread;
+    Poco::NotificationQueue _callbackQueue;
     std::atomic_size_t _clientViews;
 };
 


More information about the Libreoffice-commits mailing list