wsd/ClientSession.cpp | 2 - wsd/ClientSession.hpp | 1 wsd/DocumentBroker.cpp | 62 ++++++++++--------------------------------------- wsd/DocumentBroker.hpp | 15 ----------- 4 files changed, 15 insertions(+), 65 deletions(-)
New commits: commit d3a8106cda570fb8881139dc4347e02247036664 Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk> Date: Sat Mar 25 14:19:17 2017 -0400 wsd: remove NewSessions Sessions are now added to the DocBroker _sessions map and loaded from the poll thread first before processing their messages. Since messages are not read from the sockets outside of the poll thread, there is no reason to queue them at all. The only exception is when messages are passed directly to ClientSession during convert-to requests. That will be handled separately (for now convert-to test fails). Change-Id: I798166b9e45b5707a33d31137e01a32ce63630b1 Reviewed-on: https://gerrit.libreoffice.org/35705 Reviewed-by: Ashod Nakashian <ashnak...@gmail.com> Tested-by: Ashod Nakashian <ashnak...@gmail.com> diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp index 73e4cb5b..fbc50b98 100644 --- a/wsd/ClientSession.cpp +++ b/wsd/ClientSession.cpp @@ -38,6 +38,7 @@ ClientSession::ClientSession(const std::string& id, _uriPublic(uriPublic), _isReadOnly(readOnly), _isDocumentOwner(false), + _isLoaded(false), _stop(false) { const size_t curConnections = ++LOOLWSD::NumConnections; @@ -632,7 +633,6 @@ bool ClientSession::handleKitToClientMessage(const char* buffer, const int lengt } else if (tokens[0] == "status:") { - _isLoaded = true; docBroker->setLoaded(); // Forward the status response to the client. diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp index 19aee2af..c027f4b3 100644 --- a/wsd/ClientSession.hpp +++ b/wsd/ClientSession.hpp @@ -37,6 +37,7 @@ public: /// Returns true if a document is loaded (i.e. we got status message). bool isLoaded() const { return _isLoaded; } + void setLoaded() { _isLoaded = true; } const std::string getUserId() const { return _userId; } void setUserId(const std::string& userId) { _userId = userId; } diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index eb204b8d..fe0372e7 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -211,41 +211,20 @@ void DocumentBroker::pollThread() // Main polling loop goodness. while (!_stop && !TerminationFlag && !ShutdownRequestFlag) { - while (true) + // First, load new sessions. + for (const auto& pair : _sessions) { - std::unique_lock<std::mutex> lock(_mutex); - if (_newSessions.empty()) - break; - - NewSession& newSession = _newSessions.front(); try { - addSession(newSession._session); - - // now send the queued messages - for (std::string message : newSession._messages) - { - // provide the jailed document path to the 'load' message - assert(!_uriJailed.empty()); - std::vector<std::string> tokens = LOOLProtocol::tokenize(message); - if (tokens.size() > 1 && tokens[1] == "load") - { - // The json options must come last. - message = tokens[0] + ' ' + tokens[1] + ' ' + tokens[2]; - message += " jail=" + _uriJailed.toString() + ' '; - message += Poco::cat(std::string(" "), tokens.begin() + 3, tokens.end()); - } - - LOG_DBG("Sending a queued message: " + message); - _childProcess->sendTextFrame(message); - } + auto& session = pair.second; + if (!session->isLoaded()) + addSession(session); } catch (const std::exception& exc) { LOG_ERR("Error while adding new session to doc [" << _docKey << "]: " << exc.what()); + //TODO: Send failure to client and remove session. } - - _newSessions.pop_front(); } _poll->poll(SocketPoll::DefaultPollTimeoutMs); @@ -740,10 +719,10 @@ size_t DocumentBroker::queueSession(std::shared_ptr<ClientSession>& session) { Util::assertIsLocked(_mutex); - _newSessions.push_back(NewSession(session)); + _sessions.emplace(session->getId(), session); _poll->wakeup(); - return _sessions.size() + _newSessions.size(); + return _sessions.size(); } size_t DocumentBroker::addSession(const std::shared_ptr<ClientSession>& session) @@ -778,12 +757,9 @@ size_t DocumentBroker::addSession(const std::shared_ptr<ClientSession>& session) _markToDestroy = false; _stop = false; - const auto id = session->getId(); - if (!_sessions.emplace(id, session).second) - { - LOG_WRN("DocumentBroker: Trying to add already existing session."); - } + session->setLoaded(); + const auto id = session->getId(); const auto count = _sessions.size(); // Request a new session from the child kit. @@ -827,11 +803,6 @@ size_t DocumentBroker::removeSessionInternal(const std::string& id) { try { - // remove also from the _newSessions - _newSessions.erase(std::remove_if(_newSessions.begin(), _newSessions.end(), - [&id](NewSession& newSession) { return newSession._session->getId() == id; }), - _newSessions.end()); - Admin::instance().rmDoc(_docKey, id); auto it = _sessions.find(id); @@ -1202,15 +1173,9 @@ bool DocumentBroker::forwardToChild(const std::string& viewId, const std::string _childProcess->sendTextFrame(msg); return true; } - else - { - // try the not yet created sessions - const auto n = std::find_if(_newSessions.begin(), _newSessions.end(), [&viewId](NewSession& newSession) { return newSession._session->getId() == viewId; }); - if (n != _newSessions.end()) - n->_messages.push_back(msg); - else - LOG_WRN("Child session [" << viewId << "] not found to forward message: " << message); - } + + // try the not yet created sessions + LOG_WRN("Child session [" << viewId << "] not found to forward message: " << message); return false; } @@ -1367,7 +1332,6 @@ void DocumentBroker::dumpState(std::ostream& os) os << "\n jailed uri: " << _uriJailed.toString(); os << "\n doc key: " << _docKey; os << "\n num sessions: " << getSessionsCount(); - os << "\n new sessions: " << _newSessions.size(); os << "\n last editable?: " << _lastEditableSession; std::time_t t = std::chrono::system_clock::to_time_t( std::chrono::system_clock::now() diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp index 13acb9bb..8f670c53 100644 --- a/wsd/DocumentBroker.hpp +++ b/wsd/DocumentBroker.hpp @@ -385,21 +385,6 @@ private: Poco::Timestamp _lastFileModifiedTime; std::map<std::string, std::shared_ptr<ClientSession> > _sessions; - /// Private class to remember what sessions we have to create, and what - /// messages to send to them after they are really created. - class NewSession { - public: - NewSession(const std::shared_ptr<ClientSession>& session) : _session(session) - { - } - - std::shared_ptr<ClientSession> _session; - std::deque<std::string> _messages; - }; - - /// Sessions that are queued for addition. - std::deque<NewSession> _newSessions; - std::unique_ptr<StorageBase> _storage; std::unique_ptr<TileCache> _tileCache; std::atomic<bool> _markToDestroy; _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits