[Libreoffice-commits] online.git: 3 commits - loolwsd/LOOLKit.cpp loolwsd/LOOLSession.cpp

[Tor Lillqvist]

 loolwsd/LOOLKit.cpp     |    6 ++++--
 loolwsd/LOOLSession.cpp |   38 ++++++++++++++++++++------------------
 2 files changed, 24 insertions(+), 20 deletions(-)

New commits:
commit 7d89cfa6f0d8a1898421f0e95c98c70cdd5e5e30
Author: Tor Lillqvist <tml at collabora.com>
Date:   Mon Mar 7 19:58:26 2016 +0200

    Clean up #includes and usings.
    
    Include and use 'using' only for what is actually used. Sort the
    include and 'using' lines.

diff --git a/loolwsd/LOOLSession.cpp b/loolwsd/LOOLSession.cpp
index 5394726..32f3e7c 100644
--- a/loolwsd/LOOLSession.cpp
+++ b/loolwsd/LOOLSession.cpp
@@ -29,15 +29,12 @@
 #include <LibreOfficeKit/LibreOfficeKitEnums.h>
 
 #include <Poco/Exception.h>
-#include <Poco/File.h>
+#include <Poco/Exception.h>
+#include <Poco/Net/NetException.h>
 #include <Poco/Net/WebSocket.h>
 #include <Poco/Path.h>
 #include <Poco/String.h>
 #include <Poco/StringTokenizer.h>
-#include <Poco/URI.h>
-#include <Poco/Exception.h>
-#include <Poco/Net/NetException.h>
-#include <Poco/FileStream.h>
 
 #include "Common.hpp"
 #include "LOOLProtocol.hpp"
@@ -47,17 +44,14 @@
 
 using namespace LOOLProtocol;
 
-using Poco::File;
+using Poco::Exception;
+using Poco::IOException;
 using Poco::Net::WebSocket;
 using Poco::Path;
 using Poco::StringTokenizer;
-using Poco::Thread;
-using Poco::URI;
-using Poco::Exception;
-using Poco::IOException;
 
 LOOLSession::LOOLSession(const std::string& id, const Kind kind,
-                         std::shared_ptr<Poco::Net::WebSocket> ws) :
+                         std::shared_ptr<WebSocket> ws) :
     _kind(kind),
     _kindString(kind == Kind::ToClient ? "ToClient" :
                 kind == Kind::ToMaster ? "ToMaster" : "ToPrisoner"),
@@ -188,7 +182,7 @@ void LOOLSession::disconnect(const std::string& reason)
     }
 }
 
-bool LOOLSession::handleDisconnect(Poco::StringTokenizer& /*tokens*/)
+bool LOOLSession::handleDisconnect(StringTokenizer& /*tokens*/)
 {
     _disconnected = true;
     Util::shutdownWebSocket(_ws);
commit 9947633550e4b4bfdc3d9ae710e67317c78436a1
Author: Tor Lillqvist <tml at collabora.com>
Date:   Mon Mar 7 19:55:00 2016 +0200

    Catch IOException from sendTextFrame() in LOOLSession::disconnect()
    
    Otherwise the uncaught exception will terminate the loolkit process
    unexpectedly, which surely messes things up.

diff --git a/loolwsd/LOOLSession.cpp b/loolwsd/LOOLSession.cpp
index c28bbcc..5394726 100644
--- a/loolwsd/LOOLSession.cpp
+++ b/loolwsd/LOOLSession.cpp
@@ -54,6 +54,7 @@ using Poco::StringTokenizer;
 using Poco::Thread;
 using Poco::URI;
 using Poco::Exception;
+using Poco::IOException;
 
 LOOLSession::LOOLSession(const std::string& id, const Kind kind,
                          std::shared_ptr<Poco::Net::WebSocket> ws) :
@@ -169,14 +170,21 @@ void LOOLSession::parseDocOptions(const StringTokenizer& tokens, int& part, std:
 
 void LOOLSession::disconnect(const std::string& reason)
 {
-    if (!_disconnected)
+    try
+    {
+        if (!_disconnected)
+        {
+            if (reason != "")
+                sendTextFrame("disconnect " + reason);
+            else
+                sendTextFrame("disconnect");
+            _disconnected = true;
+            Util::shutdownWebSocket(_ws);
+        }
+    }
+    catch (const IOException& exc)
     {
-        if (reason != "")
-            sendTextFrame("disconnect " + reason);
-        else
-            sendTextFrame("disconnect");
-        _disconnected = true;
-        Util::shutdownWebSocket(_ws);
+        Log::error("LOOLSession::disconnect: Exception: " + exc.displayText() + (exc.nested() ? " (" + exc.nested()->displayText() + ")" : ""));
     }
 }
 
commit 06c6a7e1a645516bd09c060acd78050fe655562b
Author: Tor Lillqvist <tml at collabora.com>
Date:   Mon Mar 7 19:46:40 2016 +0200

    Try harder to avoid deadlock
    
    I occasionally saw a deadlock when running 'make check' where one
    thread holds the ChildProcessSession::Mutex and wants the _mutex for a
    Document, while another thread holds that _mutex and wants the
    Mutex. In particular, it is the Document::onUnload() that wants the
    _mutex. So avoid the deadlock by having Document::onUnload() first
    take the ChildProcessSession::Mutex.

diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 44184da..a109b51 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -693,8 +693,6 @@ private:
 
     void onUnload(const std::string& sessionId)
     {
-        std::unique_lock<std::recursive_mutex> lock(_mutex);
-
         const unsigned intSessionId = Util::decodeId(sessionId);
         const auto it = _connections.find(intSessionId);
         if (it == _connections.end() || !it->second || !_loKitDocument)
@@ -703,6 +701,10 @@ private:
             return;
         }
 
+        auto session = it->second->getSession();
+        auto sessionLock = session->getLock();
+        std::unique_lock<std::recursive_mutex> lock(_mutex);
+
         --_clientViews;
 
         std::ostringstream message;