[Libreoffice-commits] online.git: common/IoUtil.cpp common/IoUtil.hpp common/Session.cpp common/Session.hpp wsd/DocumentBroker.hpp wsd/LOOLWebSocket.hpp

[Ashod Nakashian]

 common/IoUtil.cpp      |   19 -------------------
 common/IoUtil.hpp      |    4 ----
 common/Session.cpp     |   29 +++++++----------------------
 common/Session.hpp     |    3 ++-
 wsd/DocumentBroker.hpp |    7 ++++++-
 wsd/LOOLWebSocket.hpp  |   33 +++++++++++++++++++++++++++++++++
 6 files changed, 48 insertions(+), 47 deletions(-)

New commits:
commit b1ff72e13f2af829e0e5e0c79739230a54a982e0
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun Jan 15 12:28:52 2017 -0500

    wsd: move shutdown web socket helper into LOOLWebSocket
    
    Change-Id: Ide377fe3a8c950490bb6c04382464f0fdfd66088
    Reviewed-on: https://gerrit.libreoffice.org/33129
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/common/IoUtil.cpp b/common/IoUtil.cpp
index cc62244..3466dca 100644
--- a/common/IoUtil.cpp
+++ b/common/IoUtil.cpp
@@ -203,25 +203,6 @@ void SocketProcessor(const std::shared_ptr<LOOLWebSocket>& ws,
             ", flags: " << std::hex << flags);
 }
 
-void shutdownWebSocket(const std::shared_ptr<LOOLWebSocket>& ws)
-{
-    try
-    {
-        // Calling LOOLWebSocket::shutdown, in case of error, would try to send a 'close' frame
-        // which won't work in case of broken pipe or timeout from peer. Just close the
-        // socket in that case preventing 'close' frame from being sent.
-        if (ws && ws->poll(Poco::Timespan(0), Socket::SelectMode::SELECT_ERROR))
-            ws->close();
-        else if (ws)
-            ws->shutdown();
-    }
-    catch (const Poco::Exception& exc)
-    {
-        LOG_WRN("Util::shutdownWebSocket: Exception: " << exc.displayText() <<
-                (exc.nested() ? " (" + exc.nested()->displayText() + ")" : ""));
-    }
-}
-
 ssize_t writeToPipe(int pipe, const char* buffer, ssize_t size)
 {
     ssize_t count = 0;
diff --git a/common/IoUtil.hpp b/common/IoUtil.hpp
index 6a81f19..416607b 100644
--- a/common/IoUtil.hpp
+++ b/common/IoUtil.hpp
@@ -27,10 +27,6 @@ namespace IoUtil
                          const std::function<void()>& closeFrame,
                          const std::function<bool()>& stopPredicate);
 
-    /// Call LOOLWebSocket::shutdown() ignoring Poco::IOException.
-    /// TODO: consider moving this directly to LOOLWebSocket
-    void shutdownWebSocket(const std::shared_ptr<LOOLWebSocket>& ws);
-
     ssize_t writeToPipe(int pipe, const char* buffer, ssize_t size);
     inline ssize_t writeToPipe(int pipe, const std::string& message)
     {
diff --git a/common/Session.cpp b/common/Session.cpp
index c409a57..75b5e4b 100644
--- a/common/Session.cpp
+++ b/common/Session.cpp
@@ -176,25 +176,17 @@ void Session::parseDocOptions(const StringTokenizer& tokens, int& part, std::str
 
 void Session::disconnect()
 {
-    try
-    {
-        if (!_disconnected)
-        {
-            _disconnected = true;
-            IoUtil::shutdownWebSocket(_ws);
-        }
-    }
-    catch (const IOException& exc)
+    if (!_disconnected)
     {
-        LOG_ERR("Session::disconnect: Exception: " << exc.displayText() <<
-                (exc.nested() ? " (" + exc.nested()->displayText() + ")" : ""));
+        _disconnected = true;
+        shutdown();
     }
 }
 
 bool Session::handleDisconnect()
 {
     _disconnected = true;
-    IoUtil::shutdownWebSocket(_ws);
+    shutdown();
     return false;
 }
 
@@ -202,16 +194,9 @@ void Session::shutdown(Poco::UInt16 statusCode, const std::string& statusMessage
 {
     if (_ws)
     {
-        try
-        {
-            LOG_TRC("Shutting down WS [" << getName() << "] with statusCode [" << statusCode << "] and reason [" << statusMessage << "].");
-            _ws->shutdown(statusCode, statusMessage);
-        }
-        catch (const Poco::Exception &exc)
-        {
-            LOG_WRN("Session::shutdown LOOLWebSocket: Exception: " <<
-                    exc.displayText() << (exc.nested() ? " (" + exc.nested()->displayText() + ")" : ""));
-        }
+        LOG_TRC("Shutting down WS [" << getName() << "] with statusCode [" <<
+                statusCode << "] and reason [" << statusMessage << "].");
+        _ws->shutdown(statusCode, statusMessage);
     }
 }
 
diff --git a/common/Session.hpp b/common/Session.hpp
index 8277c5e..d5fe524 100644
--- a/common/Session.hpp
+++ b/common/Session.hpp
@@ -51,7 +51,8 @@ public:
     /// Called to handle disconnection command from socket.
     virtual bool handleDisconnect();
 
-    void shutdown(Poco::UInt16 statusCode, const std::string& statusMessage = "");
+    void shutdown(Poco::UInt16 statusCode = Poco::Net::WebSocket::StatusCodes::WS_NORMAL_CLOSE,
+                  const std::string& statusMessage = "");
 
     bool isActive() const { return _isActive; }
     void setIsActive(bool active) { _isActive = active; }
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 0c2457a..27c4916 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -92,7 +92,12 @@ public:
         {
             LOG_DBG("Closing ChildProcess [" << _pid << "].");
             stop();
-            IoUtil::shutdownWebSocket(_ws);
+
+            if (_ws)
+            {
+                _ws->shutdown();
+            }
+
             if (_thread.joinable())
             {
                 _thread.join();
diff --git a/wsd/LOOLWebSocket.hpp b/wsd/LOOLWebSocket.hpp
index 73d8a88..81ced3a 100644
--- a/wsd/LOOLWebSocket.hpp
+++ b/wsd/LOOLWebSocket.hpp
@@ -188,6 +188,39 @@ public:
 
         return result;
     }
+
+    /// Safe shutdown by sending a normal close frame, if socket is not in error,
+    /// or, otherwise, close the socket without sending close frame, if it is.
+    void shutdown()
+    {
+        shutdown(Poco::Net::WebSocket::StatusCodes::WS_NORMAL_CLOSE);
+    }
+
+    /// Safe shutdown by sending a specific close frame, if socket is not in error,
+    /// or, otherwise, close the socket without sending close frame, if it is.
+    void shutdown(Poco::UInt16 statusCode, const std::string& statusMessage = "")
+    {
+        std::unique_lock<std::mutex> lock(_mutex);
+        try
+        {
+            // Calling shutdown, in case of error, would try to send a 'close' frame
+            // which won't work in case of broken pipe or timeout from peer. Just close the
+            // socket in that case preventing 'close' frame from being sent.
+            if (Poco::Net::WebSocket::poll(Poco::Timespan(0), Socket::SelectMode::SELECT_ERROR))
+            {
+                Poco::Net::WebSocket::close();
+            }
+            else
+            {
+                Poco::Net::WebSocket::shutdown(statusCode, statusMessage);
+            }
+        }
+        catch (const Poco::Exception& exc)
+        {
+            LOG_WRN("LOOLWebSocket::shutdown: Exception: " << exc.displayText() <<
+                    (exc.nested() ? " (" + exc.nested()->displayText() + ")" : ""));
+        }
+    }
 };
 
 #endif