[Libreoffice-commits] online.git: loolwsd/LOOLForKit.cpp loolwsd/test

[Tor Lillqvist]

 loolwsd/LOOLForKit.cpp      |   29 ++++++++++++++++++++---------
 loolwsd/test/httpwstest.cpp |   26 ++++++++++++++++++--------
 2 files changed, 38 insertions(+), 17 deletions(-)

New commits:
commit 311a748d45ec9ca6d25948641cb4446183ae86a8
Author: Tor Lillqvist <tml at collabora.com>
Date:   Mon Apr 18 14:02:36 2016 +0300

    Clean up jails of exited loolkit processes sooner
    
    In loolforkit, whenever we have forked a new loolkit, also check if
    any previously forked children have exited. Remove the jails of
    those. (The loolkit process itself does not even try to remove all of
    its jail, see 3aadd910c6e32c0e557671effa5a4c606cd6e8bf.)
    
    In order to be able to notice exited child processes in loolforkit, we
    no longer can set the action for SIGCHLD to SIG_IGN. That means that
    exiting loolkit processes will be in the zombie state until loolforkit
    picks up their exit status. As loolforkit does this check only in
    connection with forking a new child, zombie loolkit processes will
    hang around for some time, until the next loolkit process is
    forked. Not sure if this is a problem.
    
    countLoolKitProcesses() in httpwstest now needs to skip zombies.
    
    Loolwsd still takes care of removing whatever jails are left when it
    finishes.

diff --git a/loolwsd/LOOLForKit.cpp b/loolwsd/LOOLForKit.cpp
index b013c51..eee5788 100644
--- a/loolwsd/LOOLForKit.cpp
+++ b/loolwsd/LOOLForKit.cpp
@@ -18,10 +18,12 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 
+#include <atomic>
 #include <cstdlib>
 #include <cstring>
-#include <atomic>
+#include <map>
 #include <iostream>
+#include <set>
 
 #include <Poco/Path.h>
 #include <Poco/Process.h>
@@ -48,6 +50,8 @@ static bool NoCapsForKit = false;
 static std::string UnitTestLibrary;
 static std::atomic<unsigned> ForkCounter( 0 );
 
+static std::map<Process::PID, std::string> childJails;
+
 int ClientPortNumber = DEFAULT_CLIENT_PORT_NUMBER;
 
 static int pipeFd = -1;
@@ -137,11 +141,25 @@ static int createLibreOfficeKit(const std::string& childRoot,
     }
     else
     {
-        // parent
+        // Parent
+
+        // Check if some previously forked kids have died
+        Process::PID exitedChildPid;
+        int status;
+        while ((exitedChildPid = waitpid(-1, &status, WNOHANG)) > 0)
+        {
+            Log::info("Child " + std::to_string(exitedChildPid) + " has exited, removing its jail '" + childJails[exitedChildPid] + "'");
+            Util::removeFile(childJails[exitedChildPid], true);
+            childJails.erase(exitedChildPid);
+        }
+
         if (pid < 0)
             Log::syserror("Fork failed.");
         else
+        {
             Log::info("Forked kit [" + std::to_string(pid) + "].");
+            childJails[pid] = childRoot + std::to_string(pid);
+        }
 
         UnitKit::get().launchedKit(pid);
     }
@@ -177,13 +195,6 @@ int main(int argc, char** argv)
     Util::setTerminationSignals();
     Util::setFatalSignals();
 
-    // Auto-reap zombies.
-    if (signal(SIGCHLD, SIG_IGN) == SIG_ERR)
-    {
-        Log::syserror("Failed to set SIGCHLD to SIG_IGN.");
-        return Application::EXIT_SOFTWARE;
-    }
-
     std::string childRoot;
     std::string loSubPath;
     std::string sysTemplate;
diff --git a/loolwsd/test/httpwstest.cpp b/loolwsd/test/httpwstest.cpp
index 101824e..81d6a16 100644
--- a/loolwsd/test/httpwstest.cpp
+++ b/loolwsd/test/httpwstest.cpp
@@ -1084,14 +1084,24 @@ int countLoolKitProcesses()
             }
             if (pid > 1 && endPos == fileName.length())
             {
-                Poco::FileInputStream comm(procEntry.toString() + "/comm");
-                std::string command;
-                Poco::StreamCopier::copyToString(comm, command);
-                if (command.length() > 0 && command.back() == '\n')
-                    command.pop_back();
-                // std::cout << "For process " << pid << " comm is '" << command << "'" << std::endl;
-                if (command == "loolkit")
-                    result++;
+                Poco::FileInputStream stat(procEntry.toString() + "/stat");
+                std::string statString;
+                Poco::StreamCopier::copyToString(stat, statString);
+                Poco::StringTokenizer tokens(statString, " ");
+                if (tokens.count() > 3 && tokens[1] == "(loolkit)")
+                {
+                    switch (tokens[2].c_str()[0])
+                    {
+                    case 'x':
+                    case 'X': // Kinds of dead-ness.
+                    case 'Z': // zombies
+                        break; // ignore
+                    default:
+                        result++;
+                        break;
+                    }
+                    // std::cout << "Process:" << pid << ", '" << tokens[1] << "'" << " state: " << tokens[2] << std::endl;
+                }
             }
         }
         catch (const Poco::Exception&)