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

[Ashod Nakashian]

 wsd/DocumentBroker.cpp |    3 ++
 wsd/LOOLWSD.cpp        |   54 +++++++++++++++++++++++++++++--------------------
 2 files changed, 35 insertions(+), 22 deletions(-)

New commits:
commit 3d78e5f2a1a6c1369e6cdeddd39cb00a6e33d1bd
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun Mar 12 22:31:50 2017 -0400

    wsd: improved logging of extant DocBrokers after cleanup
    
    Change-Id: I4de8fde607e9e94cbadd8e100d39e0e21f2f2bdc
    Reviewed-on: https://gerrit.libreoffice.org/35126
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 25686bf..a53a7c1 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1274,6 +1274,9 @@ void DocumentBroker::terminateChild(std::unique_lock<std::mutex>& lock, const st
 
         _childProcess->close(false);
     }
+
+    // Stop the polling thread.
+    _poll->stop();
 }
 
 void DocumentBroker::closeDocument(const std::string& reason)
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 310d576..97cf845 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -261,38 +261,48 @@ bool cleanupDocBrokers()
     for (auto it = DocBrokers.begin(); it != DocBrokers.end(); )
     {
         auto docBroker = it->second;
+
+        // If document busy at the moment, cleanup later.
         auto lock = docBroker->getDeferredLock();
-        if (!lock.try_lock())
+        if (lock.try_lock())
         {
-            // Document busy at the moment, cleanup later.
-            ++it;
-            continue;
-        }
+            // Remove idle documents after 1 hour.
+            const bool idle = (docBroker->getIdleTimeSecs() >= 3600);
 
-        // Remove idle documents after 1 hour.
-        const bool idle = (docBroker->getIdleTimeSecs() >= 3600);
+            // Cleanup used and dead entries.
+            if ((docBroker->isLoaded() || docBroker->isMarkedToDestroy()) &&
+                (docBroker->getSessionsCount() == 0 || !docBroker->isAlive() || idle))
+            {
+                LOG_INF("Terminating " << (idle ? "idle" : "dead") <<
+                        " DocumentBroker for docKey [" << it->first << "].");
+                docBroker->terminateChild(lock, idle ? "idle" : "");
 
-        // Cleanup used and dead entries.
-        if (docBroker->isLoaded() &&
-            (docBroker->getSessionsCount() == 0 || !docBroker->isAlive() || idle))
-        {
-            LOG_INF("Removing " << (idle ? "idle" : "dead") <<
-                    " DocumentBroker for docKey [" << it->first << "].");
-            it = DocBrokers.erase(it);
-            docBroker->terminateChild(lock, idle ? "idle" : "");
-        }
-        else
-        {
-            ++it;
+                // Remove only when not alive.
+                if (!docBroker->isAlive())
+                {
+                    LOG_INF("Removing " << (idle ? "idle" : "dead") <<
+                            " DocumentBroker for docKey [" << it->first << "].");
+                    it = DocBrokers.erase(it);
+                    continue;
+                }
+            }
         }
+
+        ++it;
     }
 
     if (count != DocBrokers.size())
     {
-        LOG_TRC("Have " << DocBrokers.size() << " DocBrokers after cleanup.");
-        for (auto& pair : DocBrokers)
+        auto logger = Log::trace();
+        if (logger.enabled())
         {
-            LOG_TRC("DocumentBroker [" << pair.first << "].");
+            logger << "Have " << DocBrokers.size() << " DocBrokers after cleanup.\n";
+            for (auto& pair : DocBrokers)
+            {
+                logger << "DocumentBroker [" << pair.first << "].\n";
+            }
+
+            LOG_END(logger);
         }
 
         return true;