From a433e6be25f29dfdc698d52b6f334dacbc0a742d Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Thu, 26 Dec 2002 14:02:57 +0000 Subject: [PATCH] fix the zombies bug with autosave git-svn-id: svn://svn.lyx.org/lyx/lyx-devel/branches/BRANCH-1_2_X@5892 a592a061-630c-0410-9148-cb99ea01b6c8 --- src/ChangeLog | 5 ++ src/graphics/ChangeLog | 5 ++ src/graphics/GraphicsConverter.C | 5 +- src/graphics/GraphicsConverter.h | 2 +- src/lyx_cb.C | 122 ++++++++++++++++++---------- src/support/ChangeLog | 8 ++ src/support/forkedcall.C | 134 +++++++++++++++++-------------- src/support/forkedcall.h | 82 ++++++++++++------- src/support/forkedcontr.C | 7 +- src/support/forkedcontr.h | 6 +- status.12x | 3 + 11 files changed, 239 insertions(+), 140 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index 4e2127f112..9700e10839 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,8 @@ +2002-10-25 Angus Leeming + + * lyx_cb.C (AutoSave): move out child process into new class + AutoSaveBuffer. + 2002-12-18 Juergen Spitzmueller * lyxlength.[Ch]: set default unit to UNIT_NONE, diff --git a/src/graphics/ChangeLog b/src/graphics/ChangeLog index 8d81e9cca3..5e6163d165 100644 --- a/src/graphics/ChangeLog +++ b/src/graphics/ChangeLog @@ -1,3 +1,8 @@ +2002-10-25 Angus Leeming + + * GraphicsConverter.C (ConvProcess::converted): no longer receives a + string as first arg, reflecting change in ForkedCall interface. + 2002-10-18 Angus Leeming * GraphicsCacheItem.C (findTargetFormat): add debug message. diff --git a/src/graphics/GraphicsConverter.C b/src/graphics/GraphicsConverter.C index 452a362295..b8fdd67995 100644 --- a/src/graphics/GraphicsConverter.C +++ b/src/graphics/GraphicsConverter.C @@ -265,13 +265,12 @@ ConvProcess::ConvProcess(string const & script_file, int retval = call.startscript(script_command, convert_ptr); if (retval > 0) { // Unable to even start the script, so clean-up the mess! - converted(string(), 0, 1); + converted(0, 1); } } -void ConvProcess::converted(string const &/* cmd */, - pid_t /* pid */, int retval) +void ConvProcess::converted(pid_t /* pid */, int retval) { // Clean-up behind ourselves lyx::unlink(script_file_); diff --git a/src/graphics/GraphicsConverter.h b/src/graphics/GraphicsConverter.h index e2f373e1ac..628d31e176 100644 --- a/src/graphics/GraphicsConverter.h +++ b/src/graphics/GraphicsConverter.h @@ -109,7 +109,7 @@ struct ConvProcess : public SigC::Object * Cleans-up the temporary files, emits the on_finish signal and * removes the ConvProcess from the list of all processes. */ - void converted(string const & cmd, pid_t pid, int retval); + void converted(pid_t pid, int retval); /// string script_file_; diff --git a/src/lyx_cb.C b/src/lyx_cb.C index acd2dc171b..4c54a0bb97 100644 --- a/src/lyx_cb.C +++ b/src/lyx_cb.C @@ -32,6 +32,7 @@ #include "support/FileInfo.h" #include "support/filetools.h" +#include "support/forkedcall.h" #include "support/path.h" #include "support/systemcall.h" #include "support/lstrings.h" @@ -248,6 +249,82 @@ void QuitLyX() } +namespace { + +class AutoSaveBuffer : public ForkedProcess { +public: + /// + AutoSaveBuffer(BufferView & bv, string const & fname) + : bv_(bv), fname_(fname) {} + /// + virtual ForkedProcess * clone() const { + return new AutoSaveBuffer(*this); + } + /// + int start(); +private: + /// + virtual int generateChild(); + /// + BufferView & bv_; + string fname_; +}; + + +int AutoSaveBuffer::start() +{ + command_ = _("Auto-saving $$f"); + command_ = subst(command_, "$$f", fname_); + return runNonBlocking(); +} + + +int AutoSaveBuffer::generateChild() +{ + // tmp_ret will be located (usually) in /tmp + // will that be a problem? + pid_t const pid = fork(); // If you want to debug the autosave + // you should set pid to -1, and comment out the + // fork. + if (pid == 0 || pid == -1) { + // pid = -1 signifies that lyx was unable + // to fork. But we will do the save + // anyway. + bool failed = false; + + string const tmp_ret = lyx::tempName(string(), "lyxauto"); + if (!tmp_ret.empty()) { + bv_.buffer()->writeFile(tmp_ret, 1); + // assume successful write of tmp_ret + if (!lyx::rename(tmp_ret, fname_)) { + failed = true; + // most likely couldn't move between filesystems + // unless write of tmp_ret failed + // so remove tmp file (if it exists) + lyx::unlink(tmp_ret); + } + } else { + failed = true; + } + + if (failed) { + // failed to write/rename tmp_ret so try writing direct + if (!bv_.buffer()->writeFile(fname_, 1)) { + // It is dangerous to do this in the child, + // but safe in the parent, so... + if (pid == -1) + bv_.owner()->message(_("Autosave failed!")); + } + } + if (pid == 0) { // we are the child so... + _exit(0); + } + } + return pid; +} + +} // namespace anon + void AutoSave(BufferView * bv) // should probably be moved into BufferList (Lgb) @@ -265,51 +342,14 @@ void AutoSave(BufferView * bv) bv->owner()->message(_("Autosaving current document...")); // create autosave filename - string fname = bv->buffer()->filePath(); + string fname = bv->buffer()->filePath(); fname += "#"; fname += OnlyFilename(bv->buffer()->fileName()); fname += "#"; - // tmp_ret will be located (usually) in /tmp - // will that be a problem? - pid_t const pid = fork(); // If you want to debug the autosave - // you should set pid to -1, and comment out the - // fork. - if (pid == 0 || pid == -1) { - // pid = -1 signifies that lyx was unable - // to fork. But we will do the save - // anyway. - bool failed = false; - - string const tmp_ret = lyx::tempName(string(), "lyxauto"); - if (!tmp_ret.empty()) { - bv->buffer()->writeFile(tmp_ret, 1); - // assume successful write of tmp_ret - if (!lyx::rename(tmp_ret, fname)) { - failed = true; - // most likely couldn't move between filesystems - // unless write of tmp_ret failed - // so remove tmp file (if it exists) - lyx::unlink(tmp_ret); - } - } else { - failed = true; - } - - if (failed) { - // failed to write/rename tmp_ret so try writing direct - if (!bv->buffer()->writeFile(fname, 1)) { - // It is dangerous to do this in the child, - // but safe in the parent, so... - if (pid == -1) - bv->owner()->message(_("Autosave failed!")); - } - } - if (pid == 0) { // we are the child so... - _exit(0); - } - } - + AutoSaveBuffer autosave(*bv, fname); + autosave.start(); + bv->buffer()->markBakClean(); bv->owner()->resetAutosaveTimer(); } diff --git a/src/support/ChangeLog b/src/support/ChangeLog index 399b3a944a..3956c69fe1 100644 --- a/src/support/ChangeLog +++ b/src/support/ChangeLog @@ -1,3 +1,11 @@ +2002-10-25 Angus Leeming + + * forkedcall.[Ch]: split ForkedCall up into a base class ForkedProcess + and a minimal ForkedCall daughter class. + + * forkedcontr.[Ch]: minimal changes reflecting the use of a + ForkedProcess base class responsible for launching all child proceses. + 2002-12-03 Jean-Marc Lasgouttes * filetools.C (getExtFromContents): remove epsi detection diff --git a/src/support/forkedcall.C b/src/support/forkedcall.C index 25947e3898..917b8d838b 100644 --- a/src/support/forkedcall.C +++ b/src/support/forkedcall.C @@ -49,62 +49,6 @@ using std::strerror; #endif -Forkedcall::Forkedcall() - : pid_(0), retval_(0) -{} - - -int Forkedcall::startscript(Starttype wait, string const & what) -{ - if (wait == Wait) { - command_ = what; - retval_ = 0; - - pid_ = generateChild(); - if (pid_ <= 0) { // child or fork failed. - retval_ = 1; - } else { - retval_ = waitForChild(); - } - - return retval_; - } - - // DontWait - retval_ = startscript(what, SignalTypePtr()); - return retval_; -} - - -int Forkedcall::startscript(string const & what, SignalTypePtr signal) -{ - command_ = what; - signal_ = signal; - retval_ = 0; - - pid_ = generateChild(); - if (pid_ <= 0) { // child or fork failed. - retval_ = 1; - return retval_; - } - - // Non-blocking execution. - // Integrate into the Controller - ForkedcallsController & contr = ForkedcallsController::get(); - contr.addCall(*this); - - return retval_; -} - - -void Forkedcall::emitSignal() -{ - if (signal_.get()) { - signal_->emit(command_, pid_, retval_); - } -} - - namespace { class Murder : public SigC::Object { @@ -153,9 +97,56 @@ private: } // namespace anon -void Forkedcall::kill(int tol) +ForkedProcess::ForkedProcess() + : pid_(0), retval_(0) +{} + + +void ForkedProcess::emitSignal() { - lyxerr << "Forkedcall::kill(" << tol << ")" << std::endl; + if (signal_.get()) { + signal_->emit(pid_, retval_); + } +} + + +// Wait for child process to finish. +int ForkedProcess::runBlocking() +{ + retval_ = 0; + pid_ = generateChild(); + if (pid_ <= 0) { // child or fork failed. + retval_ = 1; + return retval_; + } + + retval_ = waitForChild(); + return retval_; +} + + +// Do not wait for child process to finish. +int ForkedProcess::runNonBlocking() +{ + retval_ = 0; + pid_ = generateChild(); + if (pid_ <= 0) { // child or fork failed. + retval_ = 1; + return retval_; + } + + // Non-blocking execution. + // Integrate into the Controller + ForkedcallsController & contr = ForkedcallsController::get(); + contr.addCall(*this); + + return retval_; +} + + +void ForkedProcess::kill(int tol) +{ + lyxerr << "ForkedProcess::kill(" << tol << ")" << std::endl; if (pid() == 0) { lyxerr << "Can't kill non-existent process!" << endl; return; @@ -180,7 +171,8 @@ void Forkedcall::kill(int tol) // Wait for child process to finish. Returns returncode from child. -int Forkedcall::waitForChild() { +int ForkedProcess::waitForChild() +{ // We'll pretend that the child returns 1 on all error conditions. retval_ = 1; int status; @@ -215,8 +207,30 @@ int Forkedcall::waitForChild() { } +int Forkedcall::startscript(Starttype wait, string const & what) +{ + if (wait != Wait) { + retval_ = startscript(what, SignalTypePtr()); + return retval_; + } + + command_ = what; + signal_.reset(); + return runBlocking(); +} + + +int Forkedcall::startscript(string const & what, SignalTypePtr signal) +{ + command_ = what; + signal_ = signal; + + return runNonBlocking(); +} + + // generate child in background -pid_t Forkedcall::generateChild() +int Forkedcall::generateChild() { const int MAX_ARGV = 255; char *syscmd = 0; diff --git a/src/support/forkedcall.h b/src/support/forkedcall.h index 4c24ae390f..cb6095789d 100644 --- a/src/support/forkedcall.h +++ b/src/support/forkedcall.h @@ -34,7 +34,7 @@ #include -class Forkedcall { +class ForkedProcess { public: /// enum Starttype { @@ -45,25 +45,14 @@ public: }; /// - Forkedcall(); - - /** Start the child process. - * - * The command "what" is passed to fork() for execution. - * - * There are two startscript commands available. They differ in that - * the second receives a signal that is executed on completion of - * the command. This makes sense only for a command executed - * in the background, ie DontWait. - * - * The other startscript command can be executed either blocking - * or non-blocking, but no signal will be emitted on finishing. - */ - int startscript(Starttype, string const & what); + ForkedProcess(); + /// + virtual ~ForkedProcess() {} + /// + virtual ForkedProcess * clone() const = 0; /** A SignalType signal is can be emitted once the forked process * has finished. It passes: - * the commandline string; * the PID of the child and; * the return value from the child. * @@ -71,7 +60,7 @@ public: * we can return easily to C++ methods, rather than just globally * accessible functions. */ - typedef SigC::Signal3 SignalType; + typedef SigC::Signal2 SignalType; /** The signal is connected in the calling routine to the desired * slot. We pass a shared_ptr rather than a reference to the signal @@ -83,9 +72,6 @@ public: */ typedef boost::shared_ptr SignalTypePtr; - /// - int startscript(string const & what, SignalTypePtr); - /** Invoking the following methods makes sense only if the command * is running asynchronously! */ @@ -105,6 +91,9 @@ public: */ void setRetValue(int r) { retval_ = r; } + /// Returns the identifying command (for display in the GUI perhaps). + string const & command() const { return command_; } + /** Kill child prematurely. * First, a SIGHUP is sent to the child. * If that does not end the child process within "tolerance" @@ -112,14 +101,21 @@ public: * When the child is dead, the callback is called. */ void kill(int tolerance = 5); - /// - string const & command() const { return command_; } -private: +protected: + /** Wait for child process to finish. + * Returns returncode from child. + */ + int ForkedProcess::runBlocking(); + /** Do not wait for child process to finish. + * Returns returncode from child. + */ + int ForkedProcess::runNonBlocking(); + /// Callback function SignalTypePtr signal_; - /// Commmand line + /// identifying command (for display in the GUI perhaps). string command_; /// Process ID of child @@ -127,12 +123,42 @@ private: /// Return value from child int retval_; - - /// - pid_t generateChild(); +private: + /// generate child in background + virtual int generateChild() = 0; /// Wait for child process to finish. Updates returncode from child. int waitForChild(); }; + +class Forkedcall : public ForkedProcess { +public: + /// + virtual ForkedProcess * clone() const { + return new Forkedcall(*this); + } + + /** Start the child process. + * + * The command "what" is passed to fork() for execution. + * + * There are two startscript commands available. They differ in that + * the second receives a signal that is executed on completion of + * the command. This makes sense only for a command executed + * in the background, ie DontWait. + * + * The other startscript command can be executed either blocking + * or non-blocking, but no signal will be emitted on finishing. + */ + int startscript(Starttype, string const & what); + + /// + int startscript(string const & what, SignalTypePtr); + +private: + /// + virtual int generateChild(); +}; + #endif // FORKEDCALL_H diff --git a/src/support/forkedcontr.C b/src/support/forkedcontr.C index c00db4f5fe..62d3bc1618 100644 --- a/src/support/forkedcontr.C +++ b/src/support/forkedcontr.C @@ -67,13 +67,12 @@ ForkedcallsController::~ForkedcallsController() // Add child process information to the list of controlled processes -void ForkedcallsController::addCall(Forkedcall const &newcall) +void ForkedcallsController::addCall(ForkedProcess const & newcall) { if (!timeout_->running()) timeout_->start(); - Forkedcall * call = new Forkedcall(newcall); - forkedCalls.push_back(call); + forkedCalls.push_back(newcall.clone()); childrenChanged.emit(); } @@ -86,7 +85,7 @@ void ForkedcallsController::timer() for (ListType::iterator it = forkedCalls.begin(); it != forkedCalls.end(); ++it) { - Forkedcall * actCall = *it; + ForkedProcess * actCall = *it; pid_t pid = actCall->pid(); int stat_loc; diff --git a/src/support/forkedcontr.h b/src/support/forkedcontr.h index 3b8c82d2b4..3564e60d9b 100644 --- a/src/support/forkedcontr.h +++ b/src/support/forkedcontr.h @@ -24,7 +24,7 @@ #pragma interface #endif -class Forkedcall; +class ForkedProcess; class Timeout; class ForkedcallsController : public SigC::Object { @@ -40,7 +40,7 @@ public: static ForkedcallsController & get(); /// Add a new child process to the list of controlled processes. - void addCall(Forkedcall const & newcall); + void addCall(ForkedProcess const &); /** 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 @@ -69,7 +69,7 @@ private: ForkedcallsController(ForkedcallsController const &); /// The child processes - typedef std::list ListType; + typedef std::list ListType; /// ListType forkedCalls; diff --git a/status.12x b/status.12x index d5aba49540..1e653980dc 100644 --- a/status.12x +++ b/status.12x @@ -22,6 +22,9 @@ What's new ** Bug fixes +- fix bug where autosaving would leave a lot of zombie processes after + a long time + - fix problem where it was not possible to force some values to 0 in custom margins