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

[Ashod Nakashian]

 loolwsd/DocumentBroker.cpp |   22 +++++++++++++++++++++-
 loolwsd/DocumentBroker.hpp |    5 ++++-
 loolwsd/LOOLWSD.cpp        |   33 +++++++++++++++++++++++++++++++--
 3 files changed, 56 insertions(+), 4 deletions(-)

New commits:
commit b933988a5976e1a621dde7f4d2b0f57b34cd58cd
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun Apr 17 23:29:03 2016 -0400

    loolwsd: flag and wait if document is unloading before relaoding
    
    When a new view is created on a document that is
    in the process of unloading, all sorts of things
    can go wrong. This is especially problematic when
    the document needs to be saved before unloading,
    which takes significantly longer than otherwise.
    
    Change-Id: Ib33a18cafa9d5a3a17f6bd8c6145f9331ae54044
    Reviewed-on: https://gerrit.libreoffice.org/24184
    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 7fdb7b3..75f3440 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -72,7 +72,8 @@ DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
     _childRoot(childRoot),
     _cacheRoot(getCachePath(uriPublic.toString())),
     _lastSaveTime(std::chrono::steady_clock::now()),
-    _childProcess(childProcess)
+    _childProcess(childProcess),
+    _markToDestroy(false)
 {
     assert(!_docKey.empty());
     assert(!_childRoot.empty());
@@ -102,6 +103,12 @@ bool DocumentBroker::load(const std::string& jailId)
 
     std::unique_lock<std::mutex> lock(_mutex);
 
+    if (_markToDestroy)
+    {
+        // Tearing down.
+        return false;
+    }
+
     if (_storage)
     {
         // Already loaded. Nothing to do.
@@ -307,4 +314,17 @@ size_t DocumentBroker::removeSession(const std::string& id)
     return _sessions.size();
 }
 
+bool DocumentBroker::canDestroy()
+{
+    std::unique_lock<std::mutex> lock(_mutex);
+
+    if (_sessions.size() == 1)
+    {
+        // Last view going away, can destroy.
+        _markToDestroy = true;
+    }
+
+    return _markToDestroy;
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 260a78d..2a1e73d 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -174,7 +174,9 @@ public:
     /// Removes a session by ID. Returns the new number of sessions.
     size_t removeSession(const std::string& id);
 
-    void kill() { _childProcess->close(true); };
+    // Called when the last view is going out.
+    bool canDestroy();
+    bool isMarkedToDestroy() const { return _markToDestroy; }
 
 private:
     const Poco::URI _uriPublic;
@@ -189,6 +191,7 @@ private:
     std::unique_ptr<StorageBase> _storage;
     std::unique_ptr<TileCache> _tileCache;
     std::shared_ptr<ChildProcess> _childProcess;
+    bool _markToDestroy;
     mutable std::mutex _mutex;
     std::condition_variable _saveCV;
     std::mutex _saveMutex;
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 7f833fb..0313a1a 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -494,8 +494,33 @@ private:
             Log::debug("Found DocumentBroker for docKey [" + docKey + "].");
             docBroker = it->second;
             assert(docBroker);
+
+            // If this document is going out, wait.
+            if (docBroker->isMarkedToDestroy())
+            {
+                Log::debug("Document [" + docKey + "] is marked to destroy, waiting to load.");
+                const auto timeout = POLL_TIMEOUT_MS / 2;
+                for (size_t i = 0; i < COMMAND_TIMEOUT_MS / timeout; ++i)
+                {
+                    docBrokersLock.unlock();
+                    std::this_thread::sleep_for(std::chrono::milliseconds(timeout));
+                    docBrokersLock.lock();
+                    if (docBrokers.find(docKey) == docBrokers.end())
+                    {
+                        docBroker.reset();
+                        break;
+                    }
+                }
+
+                if (docBroker)
+                {
+                    // Still here, but marked to destroy.
+                    throw std::runtime_error("Cannot load a view to document while unloading.");
+                }
+            }
         }
-        else
+
+        if (!docBroker)
         {
             // Request a kit process for this doc.
             auto child = getNewChild();
@@ -554,7 +579,11 @@ private:
             },
             []() { return TerminationFlag; });
 
-        if (docBroker->getSessionsCount() == 1 && !session->_bLoadError)
+        docBrokersLock.lock();
+        const bool canDestroy = docBroker->canDestroy();
+        docBrokersLock.unlock();
+
+        if (canDestroy && !session->_bLoadError)
         {
             Log::info("Shutdown of the last session, saving the document before tearing down.");