[Libreoffice-commits] online.git: common/IoUtil.cpp

[Ashod Nakashian]

 common/IoUtil.cpp |   31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

New commits:
commit 6b8ed0dac64dd08fab947f6b6c0af5ddbdc65a9c
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Mon Dec 12 21:48:42 2016 -0500

    loolwsd: handle socket error and close better
    
    If the socket is in error, SELECT_READ returns
    immediatly as failure. In this case we sit
    in a tight loop polling read over and over.
    We now check for SELECT_ERROR when SElECT_READ
    fails to properly close the socket and break.
    
    When receiveFrame returns -1, we should first
    check the flags for socket close, as that
    is a legitimate case of returning -ve.
    
    Change-Id: I7bbc948638a8395d28990ba92eddb9a1a9f1e6f2
    Reviewed-on: https://gerrit.libreoffice.org/31932
    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 a8cd003..9958bfe 100644
--- a/common/IoUtil.cpp
+++ b/common/IoUtil.cpp
@@ -72,6 +72,14 @@ void SocketProcessor(const std::shared_ptr<LOOLWebSocket>& ws,
             if (!ws->poll(waitTime, Poco::Net::Socket::SELECT_READ) ||
                 stopPredicate())
             {
+                // If SELECT_READ fails, it might mean the socket is in error.
+                if (ws->poll(Poco::Timespan(0), Poco::Net::Socket::SELECT_ERROR))
+                {
+                    LOG_WRN("SocketProcessor [" << name << "]: Socket error.");
+                    closeFrame();
+                    break;
+                }
+
                 // Wait some more.
                 continue;
             }
@@ -79,7 +87,7 @@ void SocketProcessor(const std::shared_ptr<LOOLWebSocket>& ws,
             try
             {
                 payload.resize(payload.capacity());
-                n = -1; // In case receiveFrame throws we log dropped data.
+                n = -1; // In case receiveFrame throws we log dropped data below.
                 (void)n;
                 n = ws->receiveFrame(payload.data(), payload.size(), flags);
                 payload.resize(std::max(n, 0));
@@ -90,17 +98,17 @@ void SocketProcessor(const std::shared_ptr<LOOLWebSocket>& ws,
                 continue;
             }
 
-            if (n == -1)
-            {
-                LOG_DBG("SocketProcessor [" << name << "]: was not an interesting frame, nothing to do here");
-                continue;
-            }
-            else if (n == 0 || ((flags & WebSocket::FRAME_OP_BITMASK) == WebSocket::FRAME_OP_CLOSE))
+            if (n == 0 || ((flags & WebSocket::FRAME_OP_BITMASK) == WebSocket::FRAME_OP_CLOSE))
             {
                 LOG_WRN("SocketProcessor [" << name << "]: Connection closed.");
                 closeFrame();
                 break;
             }
+            else if (n < 0)
+            {
+                LOG_DBG("SocketProcessor [" << name << "]: was not an interesting frame, nothing to do here");
+                continue;
+            }
 
             assert(n > 0);
 
@@ -109,17 +117,22 @@ void SocketProcessor(const std::shared_ptr<LOOLWebSocket>& ws,
             {
                 // One WS message split into multiple frames.
                 // TODO: Is this even possible with Poco if we never construct such messages outselves?
-                LOG_WRN("SocketProcessor [" << name << "]: Receiving multi-parm frame.");
+                LOG_WRN("SocketProcessor [" << name << "]: Receiving multi-part frame.");
                 while (true)
                 {
                     char buffer[READ_BUFFER_SIZE];
                     n = ws->receiveFrame(buffer, sizeof(buffer), flags);
-                    if (n <= 0 || (flags & WebSocket::FRAME_OP_BITMASK) == WebSocket::FRAME_OP_CLOSE)
+                    if (n == 0 || (flags & WebSocket::FRAME_OP_BITMASK) == WebSocket::FRAME_OP_CLOSE)
                     {
                         LOG_WRN("SocketProcessor [" << name << "]: Connection closed while reading multiframe message.");
                         closeFrame();
                         break;
                     }
+                    else if (n < 0)
+                    {
+                        LOG_DBG("SocketProcessor [" << name << "]: was not an interesting frame, nothing to do here");
+                        continue;
+                    }
 
                     payload.insert(payload.end(), buffer, buffer + n);
                     if ((flags & WebSocket::FrameFlags::FRAME_FLAG_FIN) == WebSocket::FrameFlags::FRAME_FLAG_FIN)