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

Ashod Nakashian ashod.nakashian at collabora.co.uk
Mon May 22 05:05:46 UTC 2017


 common/Session.cpp           |    3 -
 loleaflet/src/core/Socket.js |    9 +---
 wsd/DocumentBroker.cpp       |   88 ++++++++++++++++++-------------------------
 wsd/DocumentBroker.hpp       |    4 +
 4 files changed, 46 insertions(+), 58 deletions(-)

New commits:
commit 407c538f046f9245661a77e2452779c465b75087
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun May 21 19:13:55 2017 -0400

    Correctly send termination reason to clients
    
    Fixes the case when the client reconnects on idle
    disconnection (because it never got the 'close: idle'
    message).
    
    Also, show informative message to users in this case
    instead of grey screen.
    
    Change-Id: Ia2e1f2ffefe6d35dd1552e7cc44e490aab86c600
    Reviewed-on: https://gerrit.libreoffice.org/37891
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/common/Session.cpp b/common/Session.cpp
index d8756689..d29613db 100644
--- a/common/Session.cpp
+++ b/common/Session.cpp
@@ -164,8 +164,7 @@ void Session::shutdown(const WebSocketHandler::StatusCodes statusCode, const std
             static_cast<unsigned>(statusCode) << "] and reason [" << statusMessage << "].");
 
     // See protocol.txt for this application-level close frame.
-    const std::string msg = "close: " + statusMessage;
-    sendTextFrame(msg.data(), msg.size());
+    sendMessage("close: " + statusMessage);
 
     WebSocketHandler::shutdown(statusCode, statusMessage);
 }
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index 1047c4b7..4db52fef 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -228,6 +228,9 @@ L.Socket = L.Class.extend({
 			if (textMsg === 'ownertermination') {
 				msg = _('Session terminated by document owner');
 			}
+			else if (textMsg === 'idle') {
+				msg = _('Session terminated due to idleness');
+			}
 			else if (textMsg === 'shuttingdown') {
 				msg = _('Server is shutting down for maintenance (auto-saving)');
 			}
@@ -301,11 +304,7 @@ L.Socket = L.Class.extend({
 				this._map.fire('postMessage', {msgId: 'Session_Closed'});
 			}
 
-			if (textMsg === 'idle') {
-				this._map._active = false;
-			}
-
-			if (textMsg === 'ownertermination') {
+			if (textMsg === 'idle' || textMsg === 'ownertermination') {
 				this._map.remove();
 			}
 
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 956a2fe6..7d807acb 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -224,6 +224,7 @@ void DocumentBroker::pollThread()
     static const bool AutoSaveEnabled = !std::getenv("LOOL_NO_AUTOSAVE");
     static const size_t IdleDocTimeoutSecs = LOOLWSD::getConfigValue<int>(
                                                       "per_document.idle_timeout_secs", 3600);
+    std::string closeReason = "stopped";
 
     // Main polling loop goodness.
     while (!_stop && _poll->continuePolling() && !TerminationFlag)
@@ -241,30 +242,8 @@ void DocumentBroker::pollThread()
 
         if (ShutdownRequestFlag)
         {
-            // Shutting down the server: notify clients, save, and stop.
-            static const std::string msg("close: recycling");
-
-            // First copy into local container, since removeSession
-            // will erase from _sessions, but will leave the last.
-            std::map<std::string, std::shared_ptr<ClientSession>> sessions = _sessions;
-            for (const auto& pair : sessions)
-            {
-                std::shared_ptr<ClientSession> session = pair.second;
-                try
-                {
-                    // Notify the client and disconnect.
-                    session->sendMessage(msg);
-                    session->shutdown(WebSocketHandler::StatusCodes::ENDPOINT_GOING_AWAY, "recycling");
-
-                    // Remove session, save, and mark to destroy.
-                    removeSession(session->getId(), true);
-                }
-                catch (const std::exception& exc)
-                {
-                    LOG_WRN("Error while shutting down client [" <<
-                            session->getName() << "]: " << exc.what());
-                }
-            }
+            closeReason = "recycling";
+            shutdownClients(closeReason);
         }
         else if (AutoSaveEnabled && !_stop &&
                  std::chrono::duration_cast<std::chrono::seconds>(now - last30SecCheckTime).count() >= 30)
@@ -282,6 +261,7 @@ void DocumentBroker::pollThread()
         {
             LOG_INF("Terminating " << (idle ? "idle" : "dead") <<
                     " DocumentBroker for docKey [" << getDocKey() << "].");
+            closeReason = (idle ? "idle" : "dead");
             _stop = true;
         }
     }
@@ -290,11 +270,7 @@ void DocumentBroker::pollThread()
             _poll->continuePolling() << ", ShutdownRequestFlag: " << ShutdownRequestFlag <<
             ", TerminationFlag: " << TerminationFlag << ".");
 
-    // Terminate properly while we can.
-    //TODO: pass some sensible reason.
-    terminateChild("", false);
-
-    // Flush socket data.
+    // Flush socket data first.
     const int flushTimeoutMs = POLL_TIMEOUT_MS * 2; // ~1000ms
     const auto flushStartTime = std::chrono::steady_clock::now();
     while (_poll->getSocketCount())
@@ -307,6 +283,9 @@ void DocumentBroker::pollThread()
         _poll->poll(std::min(flushTimeoutMs - elapsedMs, POLL_TIMEOUT_MS / 5));
     }
 
+    // Terminate properly while we can.
+    terminateChild(closeReason, false);
+
     // Stop to mark it done and cleanup.
     _poll->stop();
     _poll->removeSockets();
@@ -1325,6 +1304,33 @@ bool DocumentBroker::forwardToClient(const std::shared_ptr<Message>& payload)
     return false;
 }
 
+void DocumentBroker::shutdownClients(const std::string& closeReason)
+{
+    assertCorrectThread();
+    LOG_INF("Terminating " << _sessions.size() << " clients of doc [" << _docKey << "].");
+
+    // First copy into local container, since removeSession
+    // will erase from _sessions, but will leave the last.
+    std::map<std::string, std::shared_ptr<ClientSession>> sessions = _sessions;
+    for (const auto& pair : sessions)
+    {
+        std::shared_ptr<ClientSession> session = pair.second;
+        try
+        {
+            // Notify the client and disconnect.
+            session->shutdown(WebSocketHandler::StatusCodes::ENDPOINT_GOING_AWAY, closeReason);
+
+            // Remove session, save, and mark to destroy.
+            removeSession(session->getId(), true);
+        }
+        catch (const std::exception& exc)
+        {
+            LOG_WRN("Error while shutting down client [" <<
+                    session->getName() << "]: " << exc.what());
+        }
+    }
+}
+
 void DocumentBroker::childSocketTerminated()
 {
     assertCorrectThread();
@@ -1336,17 +1342,7 @@ void DocumentBroker::childSocketTerminated()
 
     // We could restore the kit if this was unexpected.
     // For now, close the connections to cleanup.
-    for (auto& pair : _sessions)
-    {
-        try
-        {
-            pair.second->shutdown(WebSocketHandler::StatusCodes::ENDPOINT_GOING_AWAY, "");
-        }
-        catch (const std::exception& ex)
-        {
-            LOG_ERR("Error while terminating client connection [" << pair.first << "]: " << ex.what());
-        }
-    }
+    shutdownClients("terminated");
 }
 
 void DocumentBroker::terminateChild(const std::string& closeReason, const bool rude)
@@ -1358,17 +1354,7 @@ void DocumentBroker::terminateChild(const std::string& closeReason, const bool r
     // Close all running sessions
     if (!rude)
     {
-        for (const auto& pair : _sessions)
-        {
-            try
-            {
-                pair.second->shutdown(WebSocketHandler::StatusCodes::ENDPOINT_GOING_AWAY, closeReason);
-            }
-            catch (const std::exception& ex)
-            {
-                LOG_ERR("Error while terminating client connection [" << pair.first << "]: " << ex.what());
-            }
-        }
+        shutdownClients(closeReason);
     }
 
     if (_childProcess)
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 46481cba..14fe17e0 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -339,6 +339,10 @@ public:
     bool sendUnoSave(const std::string& sessionId, bool dontTerminateEdit = true, bool dontSaveIfUnmodified = true);
 
 private:
+
+    /// Shutdown all client connections with the given reason.
+    void shutdownClients(const std::string& closeReason);
+
     /// This gracefully terminates the connection
     /// with the child and cleans up ChildProcess etc.
     void terminateChild(const std::string& closeReason, const bool rude);


More information about the Libreoffice-commits mailing list