[Libreoffice-commits] online.git: wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp

Ashod Nakashian ashod.nakashian at collabora.co.uk
Sun Mar 26 05:06:40 UTC 2017


 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.nakashian at 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 <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at 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;


More information about the Libreoffice-commits mailing list