From a4868edb41273c82a61c3fdb8093327978272421 Mon Sep 17 00:00:00 2001
From: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Date: Sun, 1 Jan 2017 17:31:49 -0500
Subject: [PATCH] wsd: merge idle document killing with document cleanup

Since we already enumerate the DocumentBrokers and
remove dead ones before every autosave (every 30 seconds)
as well as when loading/unloading documents, there
is little reason to have a separate timeout and loop
for idle documents.

In addition, the previous code only killed the child
but didn't remove the dead DocumentBroker entry from
DocBrokers. Not clear if this was intentional, but it
would have been removed on the next cleanup anyway.

Change-Id: Ie1e09c77133165dd1255930b0b0be06fde7646b1
Reviewed-on: https://gerrit.libreoffice.org/32626
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
---
 wsd/LOOLWSD.cpp | 36 ++++++++----------------------------
 1 file changed, 8 insertions(+), 28 deletions(-)

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 2c8b8d9e9..3a07668ad 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -260,7 +260,8 @@ void alertAllUsersInternal(const std::string& msg)
 }
 }
 
-/// Remove dead DocBrokers.
+/// Remove dead and idle DocBrokers.
+/// The client of idle document should've greyed-out long ago.
 /// Returns true if at least one is removed.
 bool cleanupDocBrokers()
 {
@@ -272,11 +273,15 @@ bool cleanupDocBrokers()
         auto docBroker = it->second;
         auto lock = docBroker->getLock();
 
+        // Remove idle documents after 1 hour.
+        const bool idle = (docBroker->getIdleTime() >= 3600);
+
         // Cleanup used and dead entries.
         if (docBroker->isLoaded() &&
-            (docBroker->getSessionsCount() == 0 || !docBroker->isAlive()))
+            (docBroker->getSessionsCount() == 0 || !docBroker->isAlive() || idle))
         {
-            LOG_DBG("Removing dead DocumentBroker for docKey [" << it->first << "].");
+            LOG_INF("Removing " << (idle ? "idle" : "dead") <<
+                    " DocumentBroker for docKey [" << it->first << "].");
             it = DocBrokers.erase(it);
             docBroker->terminateChild(lock);
         }
@@ -2086,7 +2091,6 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/)
 #endif
 
     time_t last30SecCheck = time(nullptr);
-    time_t lastOneHourCheck = time(nullptr);
     int status = 0;
     while (!TerminationFlag && !SigUtil::isShuttingDown())
     {
@@ -2188,30 +2192,6 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/)
 
                 last30SecCheck = time(nullptr);
             }
-            else if (time(nullptr) >= lastOneHourCheck + 900) // Every 15 minutes
-            {
-                // Bluntly close documents that have been idle over an hour. (By that time
-                // loleaflet's greying-out has already also kicked in.)
-                try
-                {
-                    std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
-                    for (auto& pair : DocBrokers)
-                    {
-                        auto docLock = pair.second->getLock();
-                        if (pair.second->getIdleTime() >= 3600)
-                        {
-                            LOG_INF("Terminating idle document " + pair.second->getDocKey());
-                            pair.second->terminateChild(docLock);
-                        }
-                    }
-                }
-                catch (const std::exception& exc)
-                {
-                    LOG_ERR("Exception: " << exc.what());
-                }
-
-                lastOneHourCheck = time(nullptr);
-            }
             else
             {
                 // Wait if we had done no work.