[Libreoffice-commits] online.git: common/Session.hpp kit/ChildSession.cpp kit/ChildSession.hpp

[Michael Meeks (via logerrit)]

 common/Session.hpp   |    6 ------
 kit/ChildSession.cpp |   14 --------------
 kit/ChildSession.hpp |    6 ------
 3 files changed, 26 deletions(-)

New commits:
commit 701cb6a230f3796964442560e057aa05dbe01a67
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Fri Feb 14 18:49:56 2020 +0000
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Fri Feb 14 23:33:49 2020 +0100

    tdf#130673 - kill obsolete locking.
    
    This dates back to 2016 and our pre non-blocking and pre-unipoll state.
    
    It is no longer necessary - a single thread reads all data from the
    socket and feeds events into the Kit process; much cleaner.
    
    Change-Id: I46ad6806a1e0cdbb0e5cf4ea5d3e5e5078d3391a
    Reviewed-on: https://gerrit.libreoffice.org/c/online/+/88741
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/common/Session.hpp b/common/Session.hpp
index 6a3f7cfea..aa585a559 100644
--- a/common/Session.hpp
+++ b/common/Session.hpp
@@ -175,12 +175,6 @@ protected:
         _lastActivityTime = std::chrono::steady_clock::now();
     }
 
-    /// Internal lock shared with derived classes.
-    std::unique_lock<std::mutex> getLock()
-    {
-        return std::unique_lock<std::mutex>(_mutex);
-    }
-
     void dumpState(std::ostream& os) override;
 
 private:
diff --git a/kit/ChildSession.cpp b/kit/ChildSession.cpp
index d45891b58..83acd082f 100644
--- a/kit/ChildSession.cpp
+++ b/kit/ChildSession.cpp
@@ -50,8 +50,6 @@ using Poco::URI;
 
 using namespace LOOLProtocol;
 
-std::recursive_mutex ChildSession::Mutex;
-
 namespace {
 
 std::vector<unsigned char> decodeBase64(const std::string & inputBase64)
@@ -88,8 +86,6 @@ void ChildSession::disconnect()
 {
     if (!isDisconnected())
     {
-        std::unique_lock<std::recursive_mutex> lock(Mutex);
-
         if (_viewId >= 0)
         {
             if (_docManager)
@@ -125,8 +121,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?
-
         getLOKitDocument()->setView(_viewId);
 
         int curPart = 0;
@@ -610,8 +604,6 @@ bool ChildSession::loadDocument(const char * /*buffer*/, int /*length*/, const s
     assert(!getDocURL().empty());
     assert(!getJailedFilePath().empty());
 
-    std::unique_lock<std::recursive_mutex> lock(Mutex);
-
 #if defined(ENABLE_DEBUG) && !MOBILEAPP
     if (std::getenv("PAUSEFORDEBUGGER"))
     {
@@ -2219,7 +2211,6 @@ bool ChildSession::removeTextContext(const char* /*buffer*/, int /*length*/,
         return false;
     }
 
-    std::unique_lock<std::mutex> lock(getLock());
     getLOKitDocument()->setView(_viewId);
     getLOKitDocument()->removeTextContext(id, before, after);
 
@@ -2233,7 +2224,6 @@ void ChildSession::rememberEventsForInactiveUser(const int type, const std::stri
 {
     if (type == LOK_CALLBACK_INVALIDATE_TILES)
     {
-        std::unique_lock<std::mutex> lock(getLock());
         _stateRecorder.recordInvalidate(); // TODO remember the area, not just a bool ('true' invalidates everything)
     }
     else if (type == LOK_CALLBACK_INVALIDATE_VISIBLE_CURSOR ||
@@ -2249,7 +2239,6 @@ void ChildSession::rememberEventsForInactiveUser(const int type, const std::stri
              type == LOK_CALLBACK_CELL_ADDRESS ||
              type == LOK_CALLBACK_REFERENCE_MARKS)
     {
-        std::unique_lock<std::mutex> lock(getLock());
         _stateRecorder.recordEvent(type, payload);
     }
     else if (type == LOK_CALLBACK_INVALIDATE_VIEW_CURSOR ||
@@ -2259,7 +2248,6 @@ void ChildSession::rememberEventsForInactiveUser(const int type, const std::stri
              type == LOK_CALLBACK_VIEW_CURSOR_VISIBLE ||
              type == LOK_CALLBACK_VIEW_LOCK)
     {
-        std::unique_lock<std::mutex> lock(getLock());
         Poco::JSON::Parser parser;
 
         Poco::JSON::Object::Ptr root = parser.parse(payload).extract<Poco::JSON::Object::Ptr>();
@@ -2272,7 +2260,6 @@ void ChildSession::rememberEventsForInactiveUser(const int type, const std::stri
         std::string value;
         if (LOOLProtocol::parseNameValuePair(payload, name, value, '='))
         {
-            std::unique_lock<std::mutex> lock(getLock());
             _stateRecorder.recordState(name, payload);
         }
     }
@@ -2280,7 +2267,6 @@ void ChildSession::rememberEventsForInactiveUser(const int type, const std::stri
              type == LOK_CALLBACK_REDLINE_TABLE_ENTRY_MODIFIED ||
              type == LOK_CALLBACK_COMMENT)
     {
-        std::unique_lock<std::mutex> lock(getLock());
         _stateRecorder.recordEventSequence(type, payload);
     }
 }
diff --git a/kit/ChildSession.hpp b/kit/ChildSession.hpp
index 4a64be709..8aaa46b54 100644
--- a/kit/ChildSession.hpp
+++ b/kit/ChildSession.hpp
@@ -220,14 +220,12 @@ public:
     bool sendTextFrame(const char* buffer, int length) override
     {
         const auto msg = "client-" + getId() + ' ' + std::string(buffer, length);
-        const std::unique_lock<std::mutex> lock = getLock();
         return _docManager->sendFrame(msg.data(), msg.size(), WSOpCode::Text);
     }
 
     bool sendBinaryFrame(const char* buffer, int length) override
     {
         const auto msg = "client-" + getId() + ' ' + std::string(buffer, length);
-        const std::unique_lock<std::mutex> lock = getLock();
         return _docManager->sendFrame(msg.data(), msg.size(), WSOpCode::Binary);
     }
 
@@ -328,10 +326,6 @@ private:
 
     /// If we are copying to clipboard.
     bool _copyToClipboard;
-
-    /// Synchronize _loKitDocument access.
-    /// This should be owned by Document.
-    static std::recursive_mutex Mutex;
 };
 
 #endif