[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-3-0' - wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp

[Ashod Nakashian]

 wsd/DocumentBroker.cpp |   36 ++++++++++++++++++++++++------------
 wsd/DocumentBroker.hpp |    5 +++++
 2 files changed, 29 insertions(+), 12 deletions(-)

New commits:
commit 0825872b0f4089675f4e3aa904b1f5e07595a341
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Mon Jan 15 00:49:14 2018 -0500

    wsd: save before stopping
    
    We don't force saving unconditionally now. Only
    when the doc is reasonably expected to be modified
    do we force saving (to circumvent the minimum duration
    between auto-saves).
    
    We invoke auto-saving before stopping the DocBroker
    polling loop, whether due to idleness or server recycling.
    
    Change-Id: I257d55f190d3df6a3ba82f2666c7602da0581d0c
    Reviewed-on: https://gerrit.libreoffice.org/47887
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
    Tested-by: Michael Meeks <michael.meeks at collabora.com>
    (cherry picked from commit f7fc3f494ca824253a2af6ad604622bcd1340b81)
    Reviewed-on: https://gerrit.libreoffice.org/48344
    Reviewed-by: Jan Holesovsky <kendy at collabora.com>
    Tested-by: Jan Holesovsky <kendy at collabora.com>

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 03d0676b..bba4b8bb 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -275,8 +275,12 @@ void DocumentBroker::pollThread()
 
         if (ShutdownRequestFlag)
         {
-            autoSave(true);
-            stop("recycling");
+            LOG_INF("Autosaving DocumentBroker for docKey [" << getDocKey() << "] to recycle server.");
+            if (!autoSave(isPossiblyModified()))
+            {
+                LOG_INF("Terminating DocumentBroker for docKey [" << getDocKey() << "] to recycle server.");
+                stop("recycling");
+            }
         }
         else if (AutoSaveEnabled && !_stop &&
                  std::chrono::duration_cast<std::chrono::seconds>(now - last30SecCheckTime).count() >= 30)
@@ -287,14 +291,22 @@ void DocumentBroker::pollThread()
         }
 
         // Remove idle documents after 1 hour.
-        const bool idle = (getIdleTimeSecs() >= IdleDocTimeoutSecs);
-
-        // If all sessions have been removed, no reason to linger.
-        if ((isLoaded() || _markToDestroy) && (_sessions.empty() || idle))
+        const bool idle = (isLoaded() && getIdleTimeSecs() >= IdleDocTimeoutSecs);
+        if (idle)
+        {
+            // Stop if there is nothing to save.
+            LOG_INF("Autosaving idle DocumentBroker for docKey [" << getDocKey() << "] to kill.");
+            if (!autoSave(isPossiblyModified()))
+            {
+                LOG_INF("Terminating idle DocumentBroker for docKey [" << getDocKey() << "].");
+                stop("idle");
+            }
+        }
+        else if (_sessions.empty() && (isLoaded() || _markToDestroy))
         {
-            LOG_INF("Terminating " << (idle ? "idle" : "dead") <<
-                    " DocumentBroker for docKey [" << getDocKey() << "].");
-            stop(idle ? "idle" : "dead");
+            // If all sessions have been removed, no reason to linger.
+            LOG_INF("Terminating dead DocumentBroker for docKey [" << getDocKey() << "].");
+            stop("dead");
         }
     }
 
@@ -864,8 +876,8 @@ bool DocumentBroker::autoSave(const bool force)
     std::string savingSessionId;
     for (auto& sessionIt : _sessions)
     {
-        // Save the document using first session available ...
-        if (savingSessionId.empty())
+        // Save the document using an editable session, or first ...
+        if (savingSessionId.empty() || !sessionIt.second->isReadOnly())
         {
             savingSessionId = sessionIt.second->getId();
         }
@@ -1058,7 +1070,7 @@ size_t DocumentBroker::removeSession(const std::string& id)
                 ", LastEditableSession: " << lastEditableSession);
 
         // If last editable, save and don't remove until after uploading to storage.
-        if (!lastEditableSession || !autoSave(true))
+        if (!lastEditableSession || !autoSave(isPossiblyModified()))
             removeSessionInternal(id);
     }
     catch (const std::exception& ex)
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 1e8e0746..1571e426 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -367,6 +367,11 @@ private:
     /// True iff a save is in progress (requested but not completed).
     bool isSaving() const { return _lastSaveResponseTime < _lastSaveRequestTime; }
 
+    /// True if we know the doc is modified or
+    /// if there has been activity from a client after we last *requested* saving,
+    /// since there are race conditions vis-a-vis user activity while saving.
+    bool isPossiblyModified() const { return _isModified || (_lastSaveRequestTime < _lastActivityTime); }
+
     /// True iff there is at least one non-readonly session other than the given.
     /// Since only editable sessions can save, we need to use the last to
     /// save modified documents, otherwise we'll potentially have to save on