From 4515f9a333f82ebbd9431e9a5d967c29beb3816e Mon Sep 17 00:00:00 2001 From: Angus Leeming Date: Wed, 20 Apr 2005 17:35:47 +0000 Subject: [PATCH] Revert the asynchronous child process code to that of LyX 1.3.6. git-svn-id: svn://svn.lyx.org/lyx/lyx-devel/trunk@9842 a592a061-630c-0410-9148-cb99ea01b6c8 --- src/BufferView_pimpl.C | 3 +- src/ChangeLog | 6 + src/support/ChangeLog | 20 ++- src/support/filetools.C | 1 - src/support/forkedcall.C | 58 +++++-- src/support/forkedcall.h | 1 + src/support/forkedcontr.C | 347 ++++++++++++++------------------------ src/support/forkedcontr.h | 35 ++-- 8 files changed, 209 insertions(+), 262 deletions(-) diff --git a/src/BufferView_pimpl.C b/src/BufferView_pimpl.C index 716a88bcb8..5b8a3b9415 100644 --- a/src/BufferView_pimpl.C +++ b/src/BufferView_pimpl.C @@ -655,8 +655,7 @@ void BufferView::Pimpl::cursorToggle() // have finished but are waiting to communicate this fact // to the rest of LyX. ForkedcallsController & fcc = ForkedcallsController::get(); - if (fcc.processesCompleted()) - fcc.handleCompletedProcesses(); + fcc.handleCompletedProcesses(); } cursor_timeout.restart(); diff --git a/src/ChangeLog b/src/ChangeLog index ed6cd405b0..f471e172cb 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,9 @@ +2005-04-19 Angus Leeming + + * BufferView_pimpl.C (cursorToggle): no longer test whether + any child processes have been reaped before calling + handleCompletedProcesses(). + 2005-04-19 Martin Vermeer * text3.C (dispatch): fix, finally fix, the language problem in diff --git a/src/support/ChangeLog b/src/support/ChangeLog index b48b5791fc..6279bd1afd 100644 --- a/src/support/ChangeLog +++ b/src/support/ChangeLog @@ -1,3 +1,21 @@ +2005-04-19 Angus Leeming + + * filetools.C: remove unnecessary #include of forkedcontr.h. + + * forkedcall.C: protect system-specific headers with preprocessor + guards. + (running): don't call waitpid() on Windows. + (waitForChild): add Windows-specific code to wait for a child process + to finish. + (generateChild): add Windows-specific code to spawn the child in the + first place. + + * forkedcontr.[Ch]:Revert back to the LyX 1.3.x version of the code + to spawn a child process asynchronously. Do this because 'issues' + remain with the SIGCHLD handling code on *nix and because there's no + easy way to implement such a monitor on Windows. Explicit polling, + as in Lyx 1.3.x, is safe, robust and works on both platforms. + 2005-04-19 Angus Leeming * package.C.in (get_temp_dir): call GetLongPathName on Windows. @@ -8,7 +26,7 @@ on Windows. 2005-04-17 Angus Leeming - + * filetools.C (MakeDisplayPath): invoke os::external_path before returning path. diff --git a/src/support/filetools.C b/src/support/filetools.C index ab7a8d34e1..3cc0880f67 100644 --- a/src/support/filetools.C +++ b/src/support/filetools.C @@ -24,7 +24,6 @@ #include "support/convert.h" #include "support/environment.h" #include "support/filetools.h" -#include "support/forkedcontr.h" #include "support/fs_extras.h" #include "support/lstrings.h" #include "support/lyxlib.h" diff --git a/src/support/forkedcall.C b/src/support/forkedcall.C index d13f4dc3f0..419fe5aac7 100644 --- a/src/support/forkedcall.C +++ b/src/support/forkedcall.C @@ -37,17 +37,23 @@ #include -#include -#include -#include -#include -#include -#ifdef HAVE_UNISTD_H +#include + +#ifdef _WIN32 +# define SIGHUP 1 +# define SIGKILL 9 +# include +# include + +#else +# include +# include +# include # include +# include +# include #endif -#include - using std::endl; using std::string; using std::vector; @@ -152,10 +158,12 @@ bool ForkedProcess::running() const if (!pid()) return false; +#if !defined (_WIN32) // Un-UNIX like, but we don't have much use for // knowing if a zombie exists, so just reap it first. int waitstatus; waitpid(pid(), &waitstatus, WNOHANG); +#endif // Racy of course, but it will do. if (lyx::support::kill(pid(), 0) && errno == ESRCH) @@ -195,6 +203,29 @@ int ForkedProcess::waitForChild() { // We'll pretend that the child returns 1 on all error conditions. retval_ = 1; + +#if defined (_WIN32) + HANDLE const hProcess = HANDLE(pid_); + + DWORD const wait_status = ::WaitForSingleObject(hProcess, INFINITE); + + switch (wait_status) { + case WAIT_OBJECT_0: { + DWORD exit_code = 0; + if (!GetExitCodeProcess(hProcess, &exit_code)) { + lyxerr << "GetExitCodeProcess failed waiting for child\n" + << getChildErrorMessage() << std::endl; + } else + retval_ = exit_code; + break; + } + case WAIT_FAILED: + lyxerr << "WaitForSingleObject failed waiting for child\n" + << getChildErrorMessage() << std::endl; + break; + } + +#else int status; bool wait = true; while (wait) { @@ -223,6 +254,7 @@ int ForkedProcess::waitForChild() wait = false; } } +#endif return retval_; } @@ -326,7 +358,12 @@ int Forkedcall::generateChild() lyxerr << "" << std::endl; } -#ifndef __EMX__ +#if defined (__EMX__) + pid_t const cpid = spawnvp(P_SESSION|P_DEFAULT|P_MINIMIZE|P_BACKGROUND, + argv[0], &*argv.begin()); +#elif defined (_WIN32) + pid_t const cpid = spawnvp(_P_NOWAIT, argv[0], &*argv.begin()); +#else // POSIX pid_t const cpid = ::fork(); if (cpid == 0) { // Child @@ -337,9 +374,6 @@ int Forkedcall::generateChild() << strerror(errno) << endl; _exit(1); } -#else - pid_t const cpid = spawnvp(P_SESSION|P_DEFAULT|P_MINIMIZE|P_BACKGROUND, - argv[0], &*argv.begin()); #endif if (cpid < 0) { diff --git a/src/support/forkedcall.h b/src/support/forkedcall.h index d746d543e7..9b4cc892fd 100644 --- a/src/support/forkedcall.h +++ b/src/support/forkedcall.h @@ -31,6 +31,7 @@ #include + namespace lyx { namespace support { diff --git a/src/support/forkedcontr.C b/src/support/forkedcontr.C index a869bdc909..0a16b76d05 100644 --- a/src/support/forkedcontr.C +++ b/src/support/forkedcontr.C @@ -19,14 +19,23 @@ #include "debug.h" -#include +#ifdef _WIN32 +# include +# include -#include -#include -#ifdef HAVE_UNISTD_H +#else +# include +# include # include +# include + +# ifndef CXX_GLOBAL_CSTD + using std::signal; + using std::strerror; +# endif #endif -#include + +#include using boost::bind; @@ -37,149 +46,35 @@ using std::find_if; using std::string; using std::vector; -#ifndef CXX_GLOBAL_CSTD -using std::signal; -using std::strerror; -#endif - - 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 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) +#if defined(_WIN32) +string const getChildErrorMessage() { - ForkedcallsController & fcc = ForkedcallsController::get(); - - // Be safe - typedef vector::size_type size_type; - if (size_type(fcc.current_child + 1) >= fcc.reaped_children.size()) - return; - - ForkedcallsController::Data & store = - fcc.reaped_children[++fcc.current_child]; - // Clean up the child process. - store.pid = wait(&store.status); + DWORD const error_code = ::GetLastError(); + + HLOCAL t_message = 0; + bool const ok = ::FormatMessage( + FORMAT_MESSAGE_ALLOCATE_BUFFER | + FORMAT_MESSAGE_FROM_SYSTEM, + 0, error_code, + MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + (LPTSTR) &t_message, 0, 0 + ) != 0; + + std::ostringstream ss; + ss << "LyX: Error waiting for child: " << error_code; + + if (ok) { + ss << ": " << (LPTSTR)t_message; + ::LocalFree(t_message); + } else + ss << ": Error unknown."; + + return ss.str().c_str(); } - -} // namespace anon +#endif // Ensure, that only one controller exists inside process @@ -191,129 +86,137 @@ ForkedcallsController & ForkedcallsController::get() ForkedcallsController::ForkedcallsController() - : reaped_children(50), current_child(-1) -{ - signal(SIGCHLD, child_handler); - - sigemptyset(&oldMask); - sigemptyset(&newMask); - sigaddset(&newMask, SIGCHLD); -} +{} // open question: should we stop childs here? // Asger says no: I like to have my xdvi open after closing LyX. Maybe // I want to print or something. ForkedcallsController::~ForkedcallsController() -{ - signal(SIGCHLD, SIG_DFL); -} +{} void ForkedcallsController::addCall(ForkedProcess const & newcall) { 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); - } -} - - -ForkedcallsController::iterator - ForkedcallsController::find_pid(pid_t pid) -{ - return find_if(forkedCalls.begin(), forkedCalls.end(), - bind(equal_to(), - bind(&Forkedcall::pid, _1), - pid)); -} - - -// 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; - - (*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]; + ListType::iterator it = forkedCalls.begin(); + ListType::iterator end = forkedCalls.end(); + while (it != end) { + ForkedProcessPtr actCall = *it; + bool remove_it = false; - if (store.pid == -1) { - // Might happen perfectly innocently, eg as a result - // of the system (3) call. - if (errno) - lyxerr << "LyX: Error waiting for child: " - << strerror(errno) << endl; - continue; +#if defined(_WIN32) + HANDLE const hProcess = HANDLE(actCall->pid()); + + DWORD const wait_status = ::WaitForSingleObject(hProcess, 0); + + switch (wait_status) { + case WAIT_TIMEOUT: + // Still running + break; + case WAIT_OBJECT_0: { + DWORD exit_code = 0; + if (!GetExitCodeProcess(hProcess, &exit_code)) { + lyxerr << "GetExitCodeProcess failed waiting for child\n" + << getChildErrorMessage() << std::endl; + // Child died, so pretend it returned 1 + actCall->setRetValue(1); + } else { + actCall->setRetValue(exit_code); + } + remove_it = true; + break; } + case WAIT_FAILED: + lyxerr << "WaitForSingleObject failed waiting for child\n" + << getChildErrorMessage() << std::endl; + actCall->setRetValue(1); + remove_it = true; + break; + } +#else + pid_t pid = actCall->pid(); + int stat_loc; + pid_t const waitrpid = waitpid(pid, &stat_loc, WNOHANG); - ListType::iterator it = find_pid(store.pid); - if (it == forkedCalls.end()) - // Eg, child was run in blocking mode - continue; + if (waitrpid == -1) { + lyxerr << "LyX: Error waiting for child: " + << strerror(errno) << endl; - ListType::value_type child = (*it); - bool remove_it = false; + // Child died, so pretend it returned 1 + actCall->setRetValue(1); + remove_it = true; + + } else if (waitrpid == 0) { + // Still running. Move on to the next child. - if (WIFEXITED(store.status)) { + } else if (WIFEXITED(stat_loc)) { // Ok, the return value goes into retval. - child->setRetValue(WEXITSTATUS(store.status)); + actCall->setRetValue(WEXITSTATUS(stat_loc)); remove_it = true; - } else if (WIFSIGNALED(store.status)) { + } else if (WIFSIGNALED(stat_loc)) { // Child died, so pretend it returned 1 - child->setRetValue(1); + actCall->setRetValue(1); remove_it = true; - } else if (WIFSTOPPED(store.status)) { - lyxerr << "LyX: Child (pid: " << store.pid + } else if (WIFSTOPPED(stat_loc)) { + lyxerr << "LyX: Child (pid: " << pid << ") stopped on signal " - << WSTOPSIG(store.status) + << WSTOPSIG(stat_loc) << ". Waiting for child to finish." << endl; } else { lyxerr << "LyX: Something rotten happened while " - << "waiting for child " << store.pid << endl; + "waiting for child " << pid << endl; // Child died, so pretend it returned 1 - child->setRetValue(1); + actCall->setRetValue(1); remove_it = true; } +#endif if (remove_it) { - child->emitSignal(); forkedCalls.erase(it); + actCall->emitSignal(); + + /* start all over: emiting the signal can result + * in changing the list (Ab) + */ + it = forkedCalls.begin(); + } else { + ++it; } } +} - // Reset the counter - current_child = -1; - // Unblock the SIGCHLD signal and restore the old mask. - sigprocmask(SIG_SETMASK, &oldMask, 0); +ForkedcallsController::iterator +ForkedcallsController::find_pid(pid_t pid) +{ + return find_if(forkedCalls.begin(), forkedCalls.end(), + bind(equal_to(), + bind(&Forkedcall::pid, _1), + pid)); +} + + +// 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; + + (*it)->kill(tolerance); + forkedCalls.erase(it); } } // namespace support diff --git a/src/support/forkedcontr.h b/src/support/forkedcontr.h index 9e88d68289..ee1a52060b 100644 --- a/src/support/forkedcontr.h +++ b/src/support/forkedcontr.h @@ -18,9 +18,10 @@ #include -#include -//#include // needed for pid_t +#include // needed for pid_t + #include +#include #include namespace lyx { @@ -33,8 +34,8 @@ 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; } + /// Add a new child process to the list of controlled processes. + void addCall(ForkedProcess const &); /** Those child processes that are found to have finished are removed * from the list and their callback function is passed the final @@ -42,29 +43,12 @@ public: */ void handleCompletedProcesses(); - /// Add a new child process to the list of controlled processes. - void addCall(ForkedProcess const &); - /** Kill this process prematurely and remove it from the list. * The process is killed within tolerance secs. * See forkedcall.[Ch] for details. */ 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 reaped_children; - sig_atomic_t current_child; - private: ForkedcallsController(); ForkedcallsController(ForkedcallsController const &); @@ -78,11 +62,14 @@ private: /// The child processes ListType forkedCalls; - - /// Used to block SIGCHLD signals. - sigset_t newMask, oldMask; }; + +#if defined(_WIN32) +// a wrapper for GetLastError() and FormatMessage(). +std::string const getChildErrorMessage(); +#endif + } // namespace support } // namespace lyx -- 2.39.2