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

[Ashod Nakashian]

 wsd/DocumentBroker.hpp |    6 ++----
 wsd/LOOLWSD.cpp        |   18 ++++++++++++++++--
 2 files changed, 18 insertions(+), 6 deletions(-)

New commits:
commit 4ea0e2ea5f875bef644a4c94be5185a9dd03d53d
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Tue Dec 20 23:06:59 2016 -0500

    wsd: don't poll WS for write to check child alive
    
    The WS could be chocked on write, but we shouldn't
    assume the child is dead because of that. We are
    only trying to test if the child process is alive
    in that helper.
    
    Also, log DocBroker activity better.
    
    Change-Id: I108a297e43f923cab0dfa30204837dc4df15d3a1
    Reviewed-on: https://gerrit.libreoffice.org/32268
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
    Tested-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index f31471a..feb9697 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -140,17 +140,15 @@ public:
         return false;
     }
 
-    /// Check whether this child is alive and able to respond.
+    /// Check whether this child is alive and socket not in error.
     /// Note: zombies will show as alive, and sockets have waiting
     /// time after the other end-point closes. So this isn't accurate.
     bool isAlive() const
     {
         try
         {
-            using namespace Poco::Net;
             return (_pid > 1 && _ws && kill(_pid, 0) == 0 &&
-                    _ws->poll(Poco::Timespan(0), Socket::SelectMode::SELECT_WRITE) &&
-                    !_ws->poll(Poco::Timespan(0), Socket::SelectMode::SELECT_ERROR));
+                    !_ws->poll(Poco::Timespan(0), Poco::Net::Socket::SelectMode::SELECT_ERROR));
         }
         catch (const std::exception& exc)
         {
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 9b86333..76fe06c 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -278,7 +278,18 @@ bool cleanupDocBrokers()
         }
     }
 
-    return (count != DocBrokers.size());
+    if (count != DocBrokers.size())
+    {
+        LOG_TRC("Have " << DocBrokers.size() << " DocBrokers after cleanup.");
+        for (auto& pair : DocBrokers)
+        {
+            LOG_TRC("DocumentBroker [" << pair.first << "].");
+        }
+
+        return true;
+    }
+
+    return false;
 }
 
 static void forkChildren(const int number)
@@ -609,6 +620,7 @@ private:
                         LOG_DBG("Removing DocumentBroker for docKey [" << docKey << "].");
                         DocBrokers.erase(docKey);
                         docBroker->terminateChild(docLock);
+                        LOG_TRC("Have " << DocBrokers.size() << " DocBrokers after removing.");
                     }
                     else
                     {
@@ -773,7 +785,7 @@ private:
         if (it != DocBrokers.end() && it->first == docKey)
         {
             // Get the DocumentBroker from the Cache.
-            LOG_DBG("Found DocumentBroker for docKey [" << docKey << "].");
+            LOG_DBG("Found DocumentBroker with docKey [" << docKey << "].");
             docBroker = it->second;
             assert(docBroker);
 
@@ -837,6 +849,7 @@ private:
         }
 
         Util::assertIsLocked(docBrokersLock);
+        LOG_DBG("No DocumentBroker with docKey [" << docKey << "] found. New Child and Document.");
 
         if (TerminationFlag)
         {
@@ -989,6 +1002,7 @@ private:
                         LOG_INF("Removing DocumentBroker for docKey [" << docKey << "].");
                         DocBrokers.erase(docKey);
                         docBroker->terminateChild(lock);
+                        LOG_TRC("Have " << DocBrokers.size() << " DocBrokers after removing.");
                     }
                 }
             }