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 <[email protected]> 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 <[email protected]> Tested-by: Ashod Nakashian <[email protected]> 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); _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
