]> git.lyx.org Git - features.git/commitdiff
Use SIGCHLD to reap zombies.
authorAngus Leeming <leeming@lyx.org>
Wed, 24 Mar 2004 17:38:54 +0000 (17:38 +0000)
committerAngus Leeming <leeming@lyx.org>
Wed, 24 Mar 2004 17:38:54 +0000 (17:38 +0000)
git-svn-id: svn://svn.lyx.org/lyx/lyx-devel/trunk@8526 a592a061-630c-0410-9148-cb99ea01b6c8

src/BufferView_pimpl.C
src/ChangeLog
src/support/ChangeLog
src/support/forkedcontr.C
src/support/forkedcontr.h

index 79ec08c71b4b898cd0b8a11ba9f163fe4a45dbdf..2761da801af67b6fd7f1d9c01da8fe2942296bde 100644 (file)
@@ -60,6 +60,7 @@
 #include "graphics/Previews.h"
 
 #include "support/filetools.h"
+#include "support/forkedcontr.h"
 #include "support/globbing.h"
 #include "support/path_defines.h"
 #include "support/tostr.h"
@@ -72,6 +73,7 @@ using lyx::support::AddPath;
 using lyx::support::bformat;
 using lyx::support::FileFilterList;
 using lyx::support::FileSearch;
+using lyx::support::ForkedcallsController;
 using lyx::support::IsDirWriteable;
 using lyx::support::MakeDisplayPath;
 using lyx::support::strToUnsignedInt;
@@ -520,7 +522,7 @@ void BufferView::Pimpl::selectionRequested()
                sel = cur.selectionAsString(false);
                if (!sel.empty())
                        workarea().putClipboard(sel);
-       } 
+       }
 }
 
 
@@ -584,8 +586,17 @@ void BufferView::Pimpl::update()
 // Callback for cursor timer
 void BufferView::Pimpl::cursorToggle()
 {
-       if (buffer_)
+       if (buffer_) {
                screen().toggleCursor(*bv_);
+
+               // Use this opportunity to deal with any child processes that
+               // have finished but are waiting to communicate this fact
+               // to the rest of LyX.
+               ForkedcallsController & fcc = ForkedcallsController::get();
+               if (fcc.processesCompleted())
+                       fcc.handleCompletedProcesses();
+       }
+
        cursor_timeout.restart();
 }
 
@@ -862,7 +873,7 @@ bool BufferView::Pimpl::workAreaDispatch(FuncRequest const & cmd0)
                return true;
        }
 #else
-       case LFUN_MOUSE_MOTION: 
+       case LFUN_MOUSE_MOTION:
 #endif
 
        case LFUN_MOUSE_PRESS:
index e87981e5cfe201f27900599b60afca137f58740f..6324e3b575c313bb498c1854ebcc134b8ca4cda9 100644 (file)
@@ -1,3 +1,9 @@
+2004-03-24  Angus Leeming  <leeming@lyx.org>
+
+       * BufferView_pimpl.C (cursorToggle): use the cursor toggle to
+       deal with any child processes that have finished but are waiting to
+       communicate this fact to the rest of LyX.
+
 2004-03-24  Angus Leeming  <leeming@lyx.org>
 
        64-bit compile fixes.
@@ -23,7 +29,7 @@
        * lyxfunc.C (getStatus): handle read-only buffers correctly;
        handle LFUN_FILE_INSERT_*
 
-       * lyxrc.C (setDefaults, getDescription, output, read): 
+       * lyxrc.C (setDefaults, getDescription, output, read):
        * lyxrc.h: remove ps_command
 
 2004-03-22  Angus Leeming  <leeming@lyx.org>
index 5015e265293007133df84e12029bafb2bc5b44e5..d55058c51bd014cf15021664ea0e524ec4bc3e04 100644 (file)
@@ -1,3 +1,14 @@
+2004-03-24  Angus Leeming  <leeming@lyx.org>
+
+       * forkedcontr.[Ch]: get rid of the timer that we use to poll the list
+       of child proccesses and ascertain whether any have died. Instead use
+       the SIGCHLD signal emitted by the system to reap these zombies in the
+       maximally efficient manner. The subsequent emitting of the signal
+       associated with each child process *is* performed within the main
+       lyx event loop, thus ensuring that the code remains safe.
+
+       A detailed description of the design is to be found in forkedcontr.C.
+
 2004-03-24  Jean-Marc Lasgouttes  <lasgouttes@lyx.org>
 
        * filetools.C (i18nLibFileSearch): simplify the logic a bit
index e45c4002ba99c945a81a1a1f1757e39497dbe903..9a35444df4340e57bb25edcfc71e86792fe2d9e4 100644 (file)
 #include "forkedcontr.h"
 #include "forkedcall.h"
 #include "lyxfunctional.h"
-#include "debug.h"
 
-#include "frontends/Timeout.h"
+#include "debug.h"
 
-#include <boost/bind.hpp>
 #include <boost/iterator/indirect_iterator.hpp>
 
 #include <cerrno>
 #include <unistd.h>
 #include <sys/wait.h>
 
-
-using boost::bind;
-
 using std::endl;
 using std::find_if;
+
 using std::string;
+using std::vector;
 
 #ifndef CXX_GLOBAL_CSTD
+using std::signal;
 using std::strerror;
 #endif
 
@@ -44,6 +42,136 @@ using std::strerror;
 namespace lyx {
 namespace support {
 
+/* The forkedcall controller code handles finished child processes in a
+   two-stage process.
+
+   1. It uses the SIGCHLD signal emitted by the system when the child process
+      finishes to reap the resulting zombie. The handler routine also
+      updates an internal list of completed children.
+   2. The signals associated with these completed children are then emitted
+      as part of the main LyX event loop.
+
+   The guiding philosophy is that zombies are a global resource that should
+   be reaped as soon as possible whereas an internal list of dead children
+   is not. Indeed, to emit the signals within the asynchronous handler
+   routine would result in unsafe code.
+
+   The signal handler is guaranteed to be safe even though it may not be
+   atomic:
+
+   int completed_child_status;
+   sig_atomic_t completed_child_pid;
+
+   extern "C"
+   void child_handler(int)
+   {
+     // Clean up the child process.
+     completed_child_pid = wait(&completed_child_status);
+   }
+
+   (See the signals tutorial at http://tinyurl.com/3h82w.)
+
+   It's safe because:
+   1. wait(2) is guaranteed to be async-safe.
+   2. child_handler handles only SIGCHLD signals so all subsequent
+      SIGCHLD signals are blocked from entering the handler until the
+      existing signal is processed.
+
+   This handler performs 'half' of the necessary clean up after a
+   completed child process. It prevents us leaving a stream of zombies
+   behind but does not go on to tell the main LyX program to finish the
+   clean-up by emitting the stored signal. That would most definitely
+   not be safe.
+
+   The only problem with the above is that the global stores
+   completed_child_status, completed_child_pid may be overwritten before
+   the clean-up is completed in the main loop.
+
+   However, the code in child_handler can be extended to fill an array of
+   completed processes. Everything remains safe so long as no 'unsafe'
+   functions are called. (See the list of async-safe functions at
+   http://tinyurl.com/3h82w.)
+
+   struct child_data {
+     pid_t pid;
+     int status;
+   };
+
+   // This variable may need to be resized in the main program
+   // as and when a new process is forked. This resizing must be
+   // protected with sigprocmask
+   std::vector<child_data> reaped_children;
+   sig_atomic_t current_child = -1;
+
+   extern "C"
+   void child_handler(int)
+   {
+     child_data & store = reaped_children[++current_child];
+     // Clean up the child process.
+     store.pid = wait(&store.status);
+   }
+
+   That is, we build up a list of completed children in anticipation of
+   the main loop then looping over this list and invoking any associated
+   callbacks etc. The nice thing is that the main loop needs only to
+   check the value of 'current_child':
+
+   if (current_child != -1)
+     handleCompletedProcesses();
+
+   handleCompletedProcesses now loops over only those child processes
+   that have completed (ie, those stored in reaped_children). It blocks
+   any subsequent SIGCHLD signal whilst it does so:
+
+   // Used to block SIGCHLD signals.
+   sigset_t newMask, oldMask;
+
+   ForkedcallsController::ForkedcallsController()
+   {
+     reaped_children.resize(50);
+     signal(SIGCHLD, child_handler);
+
+     sigemptyset(&oldMask);
+     sigemptyset(&newMask);
+     sigaddset(&newMask, SIGCHLD);
+   }
+
+   void ForkedcallsController::handleCompletedProcesses()
+   {
+     if (current_child == -1)
+       return;
+
+     // Block the SIGCHLD signal.
+     sigprocmask(SIG_BLOCK, &newMask, &oldMask);
+
+     for (int i = 0; i != 1+current_child; ++i) {
+       child_data & store = reaped_children[i];
+       // Go on to handle the child process
+       ...
+     }
+
+     // Unblock the SIGCHLD signal and restore the old mask.
+     sigprocmask(SIG_SETMASK, &oldMask, 0);
+   }
+
+   VoilĂ ! An efficient, elegant and *safe* mechanism to handle child processes.
+*/
+
+namespace {
+
+extern "C"
+void child_handler(int)
+{
+       ForkedcallsController & fcc = ForkedcallsController::get();
+       ForkedcallsController::Data & store =
+               fcc.reaped_children[++fcc.current_child];
+       // Clean up the child process.
+       store.pid = wait(&store.status);
+}
+
+} // namespace anon
+
+
 // Ensure, that only one controller exists inside process
 ForkedcallsController & ForkedcallsController::get()
 {
@@ -53,11 +181,13 @@ ForkedcallsController & ForkedcallsController::get()
 
 
 ForkedcallsController::ForkedcallsController()
+       : reaped_children(50), current_child(-1)
 {
-       timeout_ = new Timeout(100, Timeout::ONETIME);
+       signal(SIGCHLD, child_handler);
 
-       timeout_->timeout
-               .connect(bind(&ForkedcallsController::timer, this));
+       sigemptyset(&oldMask);
+       sigemptyset(&newMask);
+       sigaddset(&newMask, SIGCHLD);
 }
 
 
@@ -66,107 +196,112 @@ ForkedcallsController::ForkedcallsController()
 // I want to print or something.
 ForkedcallsController::~ForkedcallsController()
 {
-       delete timeout_;
+       signal(SIGCHLD, SIG_DFL);
 }
 
 
 void ForkedcallsController::addCall(ForkedProcess const & newcall)
 {
-       if (!timeout_->running())
-               timeout_->start();
-
        forkedCalls.push_back(newcall.clone());
+
+       if (forkedCalls.size() > reaped_children.size()) {
+               // Block the SIGCHLD signal.
+               sigprocmask(SIG_BLOCK, &newMask, &oldMask);
+
+               reaped_children.resize(2*reaped_children.size());
+
+               // Unblock the SIGCHLD signal and restore the old mask.
+               sigprocmask(SIG_SETMASK, &oldMask, 0);
+       }
 }
 
 
-// Timer-call
-// Check the list and, if there is a stopped child, emit the signal.
-void ForkedcallsController::timer()
+ForkedcallsController::iterator ForkedcallsController::find_pid(pid_t pid)
 {
-       ListType::iterator it  = forkedCalls.begin();
-       ListType::iterator end = forkedCalls.end();
-       while (it != end) {
-               ForkedProcess * actCall = it->get();
-
-               pid_t pid = actCall->pid();
-               int stat_loc;
-               pid_t const waitrpid = waitpid(pid, &stat_loc, WNOHANG);
-               bool remove_it = false;
+       typedef boost::indirect_iterator<ListType::iterator> iterator;
+
+       iterator begin = boost::make_indirect_iterator(forkedCalls.begin());
+       iterator end   = boost::make_indirect_iterator(forkedCalls.end());
+       iterator it = find_if(begin, end,
+                             lyx::compare_memfun(&Forkedcall::pid, pid));
+
+       return it.base();
+}
+
+
+// Kill the process prematurely and remove it from the list
+// within tolerance secs
+void ForkedcallsController::kill(pid_t pid, int tolerance)
+{
+       ListType::iterator it = find_pid(pid);
+       if (it == forkedCalls.end())
+               return;
 
-               if (waitrpid == -1) {
+       (*it)->kill(tolerance);
+       forkedCalls.erase(it);
+}
+
+
+// Check the list of dead children and emit any associated signals.
+void ForkedcallsController::handleCompletedProcesses()
+{
+       if (current_child == -1)
+               return;
+
+       // Block the SIGCHLD signal.
+       sigprocmask(SIG_BLOCK, &newMask, &oldMask);
+
+       for (int i = 0; i != 1+current_child; ++i) {
+               Data & store = reaped_children[i];
+
+               if (store.pid == -1) {
                        lyxerr << "LyX: Error waiting for child: "
                               << strerror(errno) << endl;
+                       continue;
+               }
 
-                       // Child died, so pretend it returned 1
-                       actCall->setRetValue(1);
-                       remove_it = true;
+               ListType::iterator it = find_pid(store.pid);
+               BOOST_ASSERT(it != forkedCalls.end());
 
-               } else if (waitrpid == 0) {
-                       // Still running. Move on to the next child.
+               ForkedProcess & child = *it->get();
+               bool remove_it = false;
 
-               } else if (WIFEXITED(stat_loc)) {
+               if (WIFEXITED(store.status)) {
                        // Ok, the return value goes into retval.
-                       actCall->setRetValue(WEXITSTATUS(stat_loc));
+                       child.setRetValue(WEXITSTATUS(store.status));
                        remove_it = true;
 
-               } else if (WIFSIGNALED(stat_loc)) {
+               } else if (WIFSIGNALED(store.status)) {
                        // Child died, so pretend it returned 1
-                       actCall->setRetValue(1);
+                       child.setRetValue(1);
                        remove_it = true;
 
-               } else if (WIFSTOPPED(stat_loc)) {
-                       lyxerr << "LyX: Child (pid: " << pid
+               } else if (WIFSTOPPED(store.status)) {
+                       lyxerr << "LyX: Child (pid: " << store.pid
                               << ") stopped on signal "
-                              << WSTOPSIG(stat_loc)
+                              << WSTOPSIG(store.status)
                               << ". Waiting for child to finish." << endl;
 
                } else {
                        lyxerr << "LyX: Something rotten happened while "
-                               "waiting for child " << pid << endl;
+                               "waiting for child " << store.pid << endl;
 
                        // Child died, so pretend it returned 1
-                       actCall->setRetValue(1);
+                       child.setRetValue(1);
                        remove_it = true;
                }
 
                if (remove_it) {
-                       actCall->emitSignal();
+                       child.emitSignal();
                        forkedCalls.erase(it);
-
-                       /* start all over: emiting the signal can result
-                        * in changing the list (Ab)
-                        */
-                       it = forkedCalls.begin();
-               } else {
-                       ++it;
                }
        }
 
-       if (!forkedCalls.empty() && !timeout_->running()) {
-               timeout_->start();
-       }
-}
-
-
-// Kill the process prematurely and remove it from the list
-// within tolerance secs
-void ForkedcallsController::kill(pid_t pid, int tolerance)
-{
-       typedef boost::indirect_iterator<ListType::iterator> iterator;
-
-       iterator begin = boost::make_indirect_iterator(forkedCalls.begin());
-       iterator end   = boost::make_indirect_iterator(forkedCalls.end());
-       iterator it = find_if(begin, end,
-                             lyx::compare_memfun(&Forkedcall::pid, pid));
-
-       if (it == end)
-               return;
-
-       it->kill(tolerance);
-       forkedCalls.erase(it.base());
+       // Reset the counter
+       current_child = -1;
 
-       if (forkedCalls.empty())
-               timeout_->stop();
+       // Unblock the SIGCHLD signal and restore the old mask.
+       sigprocmask(SIG_SETMASK, &oldMask, 0);
 }
 
 } // namespace support
index 1ba6c78386d6206aaeeee60fe9113224ac9151e2..8768015be9dfed075934fce2347fdc7c92ab249c 100644 (file)
 #define FORKEDCONTR_H
 
 #include <boost/shared_ptr.hpp>
-#include <sys/types.h> // needed for pid_t
+#include <csignal>
+//#include <sys/types.h> // needed for pid_t
 #include <list>
-
-class Timeout;
+#include <vector>
 
 namespace lyx {
 namespace support {
@@ -32,6 +32,15 @@ public:
        /// Get hold of the only controller that can exist inside the process.
        static ForkedcallsController & get();
 
+       /// Are there any completed child processes to be cleaned-up after?
+       bool processesCompleted() const { return current_child != -1; }
+
+       /** Those child processes that are found to have finished are removed
+        *  from the list and their callback function is passed the final
+        *  return state.
+        */
+       void handleCompletedProcesses();
+
        /// Add a new child process to the list of controlled processes.
        void addCall(ForkedProcess const &);
 
@@ -41,28 +50,36 @@ public:
         */
        void kill(pid_t, int tolerance = 5);
 
+       struct Data {
+               Data() : pid(0), status(0) {}
+               pid_t pid;
+               int status;
+       };
+
+       /** These data are used by the SIGCHLD handler to populate a list
+        *  of child processes that have completed and been reaped.
+        *  The associated signals are then emitted within the main LyX
+        *  event loop.
+        */
+       std::vector<Data> reaped_children;
+       sig_atomic_t current_child;
+
 private:
        ForkedcallsController();
        ForkedcallsController(ForkedcallsController const &);
        ~ForkedcallsController();
 
-       /** This method is connected to the timer. Every XX ms it is called
-        *  so that we can check on the status of the children. Those that
-        *  are found to have finished are removed from the list and their
-        *  callback function is passed the final return state.
-        */
-       void timer();
-
-       /// The child processes
        typedef boost::shared_ptr<ForkedProcess> ForkedProcessPtr;
        typedef std::list<ForkedProcessPtr> ListType;
-       ///
+       typedef ListType::iterator iterator;
+
+       iterator find_pid(pid_t);
+
+       /// The child processes
        ListType forkedCalls;
 
-       /** The timer. Enables us to check the status of the children
-        *  every XX ms and to invoke a callback on completion.
-        */
-       Timeout * timeout_;
+       /// Used to block SIGCHLD signals.
+       sigset_t newMask, oldMask;
 };
 
 } // namespace support