[Libreoffice-commits] online.git: wsd/LOOLWSD.cpp

[Ashod Nakashian]

 wsd/LOOLWSD.cpp |   73 +++++++++++++++++++++++++++-----------------------------
 1 file changed, 36 insertions(+), 37 deletions(-)

New commits:
commit a31e606a98daf5b03e9fbd85484b4d269322fbed
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Tue Jan 10 00:38:16 2017 -0500

    wsd: retry loading DocumentBroker and adding session
    
    In some cases when the last session is destroying
    and we expect the DocumentBroker to be removed,
    while waiting for autosave, a new client might
    connect to the very same document.
    
    In those cases we shouldn't fail but should retry
    loading the document again once it has been unloaded.
    
    Change-Id: I97ebb73991b9739cfb4e93b7de9115bdb48fda7a
    Reviewed-on: https://gerrit.libreoffice.org/33125
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 889b817..7db7e02 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -832,31 +832,42 @@ private:
                 }
             }
 
-            auto docBroker = findOrCreateDocBroker(docKey, ws, id, uriPublic);
-            if (docBroker)
+            int retry = 3;
+            while (retry-- > 0)
             {
-                auto session = createNewClientSession(uri, ws, id, uriPublic, docBroker, isReadOnly);
-                if (session)
+                auto docBroker = findOrCreateDocBroker(docKey, ws, id, uriPublic);
+                if (docBroker)
                 {
-                    // Process the request in an exception-safe way.
-                    processGetRequest(uri, ws, id, docBroker, session);
+                    auto session = createNewClientSession(uri, ws, id, uriPublic, docBroker, isReadOnly);
+                    if (session)
+                    {
+                        // Process the request in an exception-safe way.
+                        processGetRequest(uri, ws, id, docBroker, session);
+                        break;
+                    }
+                }
+
+                if (retry > 0)
+                {
+                    LOG_WRN("Failed to connect DocBroker and Client Session, retrying.");
+                }
+                else
+                {
+                    const std::string msg = SERVICE_UNAVAILABLE_INTERNAL_ERROR;
+                    LOG_ERR("handleGetRequest: Giving up trying to connect client: " << msg);
+                    try
+                    {
+                        ws->sendFrame(msg.data(), msg.size());
+                        // abnormal close frame handshake
+                        ws->shutdown(WebSocket::WS_ENDPOINT_GOING_AWAY);
+                    }
+                    catch (const std::exception& exc2)
+                    {
+                        LOG_ERR("handleGetRequest: exception while sending WS error message [" << msg << "]: " << exc2.what());
+                    }
+
+                    break;
                 }
-            }
-        }
-        catch (const WebSocketErrorMessageException& exc)
-        {
-            // Internal error that should be passed on to the client.
-            const auto msg = exc.toString();
-            LOG_ERR("handleGetRequest: WebSocketErrorMessageException: " << msg);
-            try
-            {
-                ws->sendFrame(msg.data(), msg.size());
-                // abnormal close frame handshake
-                ws->shutdown(WebSocket::WS_ENDPOINT_GOING_AWAY);
-            }
-            catch (const std::exception& exc2)
-            {
-                LOG_ERR("handleGetRequest: exception while sending WS error message [" << msg << "]: " << exc2.what());
             }
         }
         catch (const std::exception& exc)
@@ -980,16 +991,6 @@ private:
             docBroker = createNewDocBroker(docKey, ws, uriPublic);
         }
 
-        // Validate the broker.
-        if (!docBroker || !docBroker->isAlive())
-        {
-            LOG_ERR("DocBroker is invalid or premature termination of child "
-                    "process. Service Unavailable.");
-            DocBrokers.erase(docKey);
-
-            throw WebSocketErrorMessageException(SERVICE_UNAVAILABLE_INTERNAL_ERROR);
-        }
-
         return docBroker;
     }
 
@@ -1013,7 +1014,7 @@ private:
         {
             // Let the client know we can't serve now.
             LOG_ERR("Failed to get new child. Service Unavailable.");
-            throw WebSocketErrorMessageException(SERVICE_UNAVAILABLE_INTERNAL_ERROR);
+            return nullptr;
         }
 
         // Set the one we just created.
@@ -1042,6 +1043,7 @@ private:
             if (!docBroker->isAlive())
             {
                 LOG_ERR("DocBroker is invalid or premature termination of child process.");
+                lock.unlock();
                 removeDocBrokerSession(docBroker);
                 return nullptr;
             }
@@ -1049,6 +1051,7 @@ private:
             if (docBroker->isMarkedToDestroy())
             {
                 LOG_ERR("DocBroker is marked to destroy, can't add session.");
+                lock.unlock();
                 removeDocBrokerSession(docBroker);
                 return nullptr;
             }
@@ -1162,10 +1165,6 @@ private:
             LOOLWSD::dumpEventTrace(docBroker->getJailId(), id, "EndSession: " + uri);
             LOG_INF("Finishing GET request handler for session [" << id << "].");
         }
-        catch (const WebSocketErrorMessageException&)
-        {
-            throw;
-        }
         catch (const UnauthorizedRequestException& exc)
         {
             LOG_ERR("Error in client request handler: " << exc.toString());