[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp

[Ashod Nakashian]

 loolwsd/DocumentBroker.cpp |   52 ++++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 24 deletions(-)

New commits:
commit f1f1ff7057871e27be98b3fe6a92ad4d8a72608d
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun Nov 6 22:11:59 2016 -0500

    loolwsd: correct document loading and error handling
    
    Change-Id: I7529c8584356f5eea3c91b5d5ffdd6a956710241
    Reviewed-on: https://gerrit.libreoffice.org/30640
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 1613aec..f18b049 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -462,33 +462,21 @@ size_t DocumentBroker::addSession(std::shared_ptr<ClientSession>& session)
     const auto id = session->getId();
     const std::string aMessage = "session " + id + " " + _docKey;
 
-    try
-    {
-        std::lock_guard<std::mutex> lock(_mutex);
-
-        // Request a new session from the child kit.
-        _childProcess->sendTextFrame(aMessage);
+    std::lock_guard<std::mutex> lock(_mutex);
 
-        auto ret = _sessions.emplace(id, session);
-        if (!ret.second)
-        {
-            LOG_WRN("DocumentBroker: Trying to add already existing session.");
-        }
+    auto ret = _sessions.emplace(id, session);
+    if (!ret.second)
+    {
+        LOG_WRN("DocumentBroker: Trying to add already existing session.");
+    }
 
-        if (session->isReadOnly())
+    try
+    {
+        // First load the document, since this can fail.
+        if (!load(id, std::to_string(_childProcess->getPid())))
         {
-            LOG_DBG("Adding a readonly session [" << id << "]");
-        }
+            _sessions.erase(id);
 
-        // Below values are recalculated when startDestroy() is called (before destroying the
-        // document). It is safe to reset their values to their defaults whenever a new session is added.
-        _lastEditableSession = false;
-        _markToDestroy = false;
-
-        bool loaded;
-        loaded = load(id, std::to_string(_childProcess->getPid()));
-        if (!loaded)
-        {
             const auto msg = "Failed to load document with URI [" + session->getPublicUri().toString() + "].";
             LOG_ERR(msg);
             throw std::runtime_error(msg);
@@ -496,6 +484,9 @@ size_t DocumentBroker::addSession(std::shared_ptr<ClientSession>& session)
     }
     catch (const StorageSpaceLowException&)
     {
+        LOG_ERR("Out of storage while loading document with URI [" << session->getPublicUri().toString() << "].");
+        _sessions.erase(id);
+
         // 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,
@@ -504,6 +495,19 @@ size_t DocumentBroker::addSession(std::shared_ptr<ClientSession>& session)
         throw;
     }
 
+    // Below values are recalculated when startDestroy() is called (before destroying the
+    // document). It is safe to reset their values to their defaults whenever a new session is added.
+    _lastEditableSession = false;
+    _markToDestroy = false;
+
+    // Request a new session from the child kit.
+    _childProcess->sendTextFrame(aMessage);
+
+    if (session->isReadOnly())
+    {
+        LOG_DBG("Adding a readonly session [" << id << "]");
+    }
+
     // Tell the admin console about this new doc
     Admin::instance().addDoc(_docKey, getPid(), getFilename(), id);
 
@@ -538,7 +542,7 @@ size_t DocumentBroker::removeSession(const std::string& id)
 
 void DocumentBroker::alertAllUsersOfDocument(const std::string& cmd, const std::string& kind)
 {
-    std::lock_guard<std::mutex> lock(_mutex);
+    Util::assertIsLocked(_mutex);
 
     std::stringstream ss;
     ss << "error: cmd=" << cmd << " kind=" << kind;