[Libreoffice-commits] online.git: loolwsd/ChildSession.cpp loolwsd/ChildSession.hpp loolwsd/ClientSession.cpp loolwsd/ClientSession.hpp loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLKit.cpp loolwsd/LOOLSession.cpp loolwsd/LOOLWSD.cpp loolwsd/PrisonerSession.cpp loolwsd/PrisonerSession.hpp

Ashod Nakashian ashod.nakashian at collabora.co.uk
Sun Oct 16 22:04:50 UTC 2016


 loolwsd/ChildSession.cpp    |    3 +--
 loolwsd/ChildSession.hpp    |    1 -
 loolwsd/ClientSession.cpp   |   18 +++++-------------
 loolwsd/ClientSession.hpp   |    4 ++--
 loolwsd/DocumentBroker.cpp  |   27 +++++++++++++++++++++++----
 loolwsd/DocumentBroker.hpp  |    2 +-
 loolwsd/LOOLKit.cpp         |   16 ++--------------
 loolwsd/LOOLSession.cpp     |    5 -----
 loolwsd/LOOLWSD.cpp         |   24 ++++++++++++------------
 loolwsd/PrisonerSession.cpp |    3 +--
 loolwsd/PrisonerSession.hpp |    1 -
 11 files changed, 47 insertions(+), 57 deletions(-)

New commits:
commit b44d71f0ae9016e8c47bb911905f80c4033afd52
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Wed Oct 12 23:04:07 2016 -0400

    loolwsd: no need to create prisoner WS anymore
    
    Change-Id: Iaebac49f47353a6848fd2232a1838e4eaadaeefc
    Reviewed-on: https://gerrit.libreoffice.org/29937
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/ChildSession.cpp b/loolwsd/ChildSession.cpp
index cf2115c..15fe4c5 100644
--- a/loolwsd/ChildSession.cpp
+++ b/loolwsd/ChildSession.cpp
@@ -35,10 +35,9 @@ using namespace LOOLProtocol;
 std::recursive_mutex ChildSession::Mutex;
 
 ChildSession::ChildSession(const std::string& id,
-                           std::shared_ptr<WebSocket> ws,
                            const std::string& jailId,
                            IDocumentManager& docManager) :
-    LOOLSession(id, Kind::ToMaster, ws),
+    LOOLSession(id, Kind::ToMaster, nullptr),
     _jailId(jailId),
     _docManager(docManager),
     _viewId(-1),
diff --git a/loolwsd/ChildSession.hpp b/loolwsd/ChildSession.hpp
index d8d68bd..c901cf0 100644
--- a/loolwsd/ChildSession.hpp
+++ b/loolwsd/ChildSession.hpp
@@ -67,7 +67,6 @@ public:
     /// jailId The JailID of the jail root directory,
     //         used by downloadas to construct jailed path.
     ChildSession(const std::string& id,
-                 std::shared_ptr<Poco::Net::WebSocket> ws,
                  const std::string& jailId,
                  IDocumentManager& docManager);
     virtual ~ChildSession();
diff --git a/loolwsd/ClientSession.cpp b/loolwsd/ClientSession.cpp
index 37f7f45..31df240 100644
--- a/loolwsd/ClientSession.cpp
+++ b/loolwsd/ClientSession.cpp
@@ -176,15 +176,6 @@ bool ClientSession::_handleInput(const char *buffer, int length)
     }
     else
     {
-        // All other commands are such that they always require a
-        // LibreOfficeKitDocument session, i.e. need to be handled in
-        // a child process.
-        if (_peer.expired())
-        {
-            Log::error(getName() + " has no peer to handle [" + tokens[0] + "].");
-            return false;
-        }
-
         // Allow 'downloadas' for all kinds of views
         if ( (isReadOnly()) && tokens[0] != "downloadas" &&
              tokens[0] != "userinactive" && tokens[0] != "useractive")
@@ -352,12 +343,13 @@ bool ClientSession::sendCombinedTiles(const char* /*buffer*/, int /*length*/, St
 
 bool ClientSession::shutdownPeer(Poco::UInt16 statusCode)
 {
-    auto peer = _peer.lock();
-    if (peer && !peer->isCloseFrame())
+    if (_peer && !_peer->isCloseFrame())
     {
-        peer->shutdown(statusCode);
+        _peer->shutdown(statusCode);
+        return true;
     }
-    return peer != nullptr;
+
+    return false;
 }
 
 bool ClientSession::forwardToChild(const char *buffer, int length)
diff --git a/loolwsd/ClientSession.hpp b/loolwsd/ClientSession.hpp
index 6f76bb1..906a430 100644
--- a/loolwsd/ClientSession.hpp
+++ b/loolwsd/ClientSession.hpp
@@ -33,7 +33,7 @@ public:
     bool isReadOnly() const { return _isReadOnly; }
 
     void setPeer(const std::shared_ptr<PrisonerSession>& peer) { _peer = peer; }
-    std::shared_ptr<PrisonerSession> getPeer() const { return _peer.lock(); }
+    std::shared_ptr<PrisonerSession> getPeer() const { return _peer; }
     bool shutdownPeer(Poco::UInt16 statusCode);
 
     void setUserName(const std::string& userName) { _userName = userName; }
@@ -86,7 +86,7 @@ private:
     bool _isReadOnly;
 
     /// Our peer that connects us to the child.
-    std::weak_ptr<PrisonerSession> _peer;
+    std::shared_ptr<PrisonerSession> _peer;
 
     /// Store URLs of completed 'save as' documents.
     MessageQueue _saveAsQueue;
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index adadee2..fd29bf5 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -160,7 +160,6 @@ bool DocumentBroker::load(const std::string& sessionId, const std::string& jailI
         return false;
     }
 
-    std::unique_lock<std::mutex> lock(_mutex);
     auto it = _sessions.find(sessionId);
     if (it == _sessions.end())
     {
@@ -382,7 +381,7 @@ bool DocumentBroker::sendUnoSave(const bool dontSaveIfUnmodified)
         const auto saveArgs = oss.str();
         Log::trace(".uno:Save arguments: " + saveArgs);
         const auto command = "uno .uno:Save " + saveArgs;
-        sessionIt.second->handleInput(command.data(), command.size());
+        forwardToChild(sessionIt.second->getId(), command.data(), command.size());
         return true;
     }
 
@@ -423,6 +422,28 @@ size_t DocumentBroker::addSession(std::shared_ptr<ClientSession>& session)
     _lastEditableSession = false;
     _markToDestroy = false;
 
+    try
+    {
+        load(id, std::to_string(_childProcess->getPid()));
+    }
+    catch (const StorageSpaceLowException&)
+    {
+        // We use the same message as is sent when some of lool's own locations are full,
+        // even if in this case it might be a totally different location (file system, or
+        // some other type of storage somewhere). This message is not sent to all clients,
+        // though, just to all sessions of this document.
+        alertAllUsersOfDocument("internal", "diskfull");
+        throw;
+    }
+
+    auto prisonerSession = std::make_shared<PrisonerSession>(id, shared_from_this());
+
+    // Connect the prison session to the client.
+    if (!connectPeers(prisonerSession))
+    {
+        Log::warn("Failed to connect " + session->getName() + " to its peer.");
+    }
+
     return _sessions.size();
 }
 
@@ -430,8 +451,6 @@ bool DocumentBroker::connectPeers(std::shared_ptr<PrisonerSession>& session)
 {
     const auto id = session->getId();
 
-    std::lock_guard<std::mutex> lock(_mutex);
-
     auto it = _sessions.find(id);
     if (it != _sessions.end())
     {
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 5d0b414..6891507 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -143,7 +143,7 @@ class ClientSession;
 /// in jail and brokering loading it from Storage
 /// and saving it back.
 /// Contains URI, physical path, etc.
-class DocumentBroker
+class DocumentBroker : public std::enable_shared_from_this<DocumentBroker>
 {
 public:
 
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index cd62e12..172da5c 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -330,19 +330,7 @@ public:
                         << " view for url: " << _url << " for sessionId: " << sessionId
                         << " on jailId: " << _jailId << Log::end;
 
-            // Open websocket connection between the child process and the
-            // parent. The parent forwards us requests that it can't handle (i.e most).
-            HTTPClientSession cs("127.0.0.1", MasterPortNumber);
-            cs.setTimeout(0);
-            const auto childUrl = std::string(CHILD_URI) + "sessionId=" + sessionId + "&jailId=" + _jailId + "&docKey=" + _docKey;
-            HTTPRequest request(HTTPRequest::HTTP_GET, childUrl);
-            HTTPResponse response;
-
-            auto ws = std::make_shared<WebSocket>(cs, request, response);
-            ws->setReceiveTimeout(0);
-
-            auto session = std::make_shared<ChildSession>(sessionId, ws, _jailId, *this);
-
+            auto session = std::make_shared<ChildSession>(sessionId, _jailId, *this);
             if (!_sessions.emplace(sessionId, session).second)
             {
                 Log::error("Session already exists for child: " + _jailId + ", session: " + sessionId);
@@ -1519,7 +1507,7 @@ void lokit_main(const std::string& childRoot,
 
                         // Validate and create session.
                         if (!(url == document->getUrl() &&
-                            document->createSession(sessionId)))
+                              document->createSession(sessionId)))
                         {
                             Log::debug("CreateSession failed.");
                         }
diff --git a/loolwsd/LOOLSession.cpp b/loolwsd/LOOLSession.cpp
index 3b2c0d3..810a3fe 100644
--- a/loolwsd/LOOLSession.cpp
+++ b/loolwsd/LOOLSession.cpp
@@ -65,11 +65,6 @@ LOOLSession::LOOLSession(const std::string& id, const Kind kind,
     _haveDocPassword(false),
     _isDocPasswordProtected(false)
 {
-    // Only a post request can have a null ws.
-    if (_kind != Kind::ToClient)
-    {
-        assert(_ws && "Expected valid web-socket but got null.");
-    }
 }
 
 LOOLSession::~LOOLSession()
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index f9938c0..32354a5 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -813,9 +813,6 @@ private:
             Log::trace("Sending to Client [" + status + "].");
             ws->sendFrame(status.data(), (int) status.size());
 
-            // Wait until the client has connected with a prison socket.
-            waitBridgeCompleted(session);
-
             LOOLWSD::dumpEventTrace(docBroker->getJailId(), id, "NewSession: " + uri);
 
             // Now the bridge beetween the client and kit process is connected
@@ -844,6 +841,7 @@ private:
                 []() { return !!TerminationFlag; });
 
             // Connection terminated. Destroy session.
+            Log::debug("Client session [" + id + "] terminated. Cleaning up.");
             {
                 std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
 
@@ -924,6 +922,8 @@ private:
             ws->shutdown(WebSocket::WS_ENDPOINT_GOING_AWAY);
             session->shutdownPeer(WebSocket::WS_ENDPOINT_GOING_AWAY);
         }
+
+        Log::info("Finished GET request handler for session [" + id + "].");
     }
 
     /// Sends back the WOPI Discovery XML.
@@ -1159,15 +1159,15 @@ public:
                 }
             }
 
-            if (pid <= 0)
-            {
-                Log::error("Invalid PID in child URI [" + request.getURI() + "].");
-                return;
-            }
+        if (pid <= 0)
+        {
+            Log::error("Invalid PID in child URI [" + request.getURI() + "].");
+            return;
+        }
 
-            Log::info("New child [" + std::to_string(pid) + "].");
-            auto ws = std::make_shared<WebSocket>(request, response);
-            UnitWSD::get().newChild(ws);
+        Log::info("New child [" + std::to_string(pid) + "].");
+        auto ws = std::make_shared<WebSocket>(request, response);
+        UnitWSD::get().newChild(ws);
 
             addNewChild(std::make_shared<ChildProcess>(pid, ws));
             return;
@@ -1239,7 +1239,7 @@ public:
             }
 
             auto ws = std::make_shared<WebSocket>(request, response);
-            auto session = std::make_shared<PrisonerSession>(sessionId, ws, docBroker);
+            auto session = std::make_shared<PrisonerSession>(sessionId, docBroker);
 
             // Connect the prison session to the client.
             if (!docBroker->connectPeers(session))
diff --git a/loolwsd/PrisonerSession.cpp b/loolwsd/PrisonerSession.cpp
index 09783d1..30b76fa 100644
--- a/loolwsd/PrisonerSession.cpp
+++ b/loolwsd/PrisonerSession.cpp
@@ -34,9 +34,8 @@ using Poco::Path;
 using Poco::StringTokenizer;
 
 PrisonerSession::PrisonerSession(const std::string& id,
-                                 std::shared_ptr<Poco::Net::WebSocket> ws,
                                  std::shared_ptr<DocumentBroker> docBroker) :
-    LOOLSession(id, Kind::ToPrisoner, ws),
+    LOOLSession(id, Kind::ToPrisoner, nullptr),
     _docBroker(std::move(docBroker)),
     _curPart(0)
 {
diff --git a/loolwsd/PrisonerSession.hpp b/loolwsd/PrisonerSession.hpp
index bcd386a..3d6ac6c 100644
--- a/loolwsd/PrisonerSession.hpp
+++ b/loolwsd/PrisonerSession.hpp
@@ -20,7 +20,6 @@ class PrisonerSession final : public LOOLSession, public std::enable_shared_from
 {
 public:
     PrisonerSession(const std::string& id,
-                    std::shared_ptr<Poco::Net::WebSocket> ws,
                     std::shared_ptr<DocumentBroker> docBroker);
 
     virtual ~PrisonerSession();


More information about the Libreoffice-commits mailing list