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

@@ -1,3 +1,8 @@
+2002-10-25  Angus Leeming  <leeming@lyx.org>
+
+	* lyx_cb.C (AutoSave): move out child process into new class
+	AutoSaveBuffer.
+
 2002-12-18  Juergen Spitzmueller  <j.spitzmueller@gmx.de>
 
 	* lyxlength.[Ch]: set default unit to UNIT_NONE,

src/graphics/ChangeLog

@@ -1,3 +1,8 @@
+2002-10-25  Angus Leeming  <leeming@lyx.org>
+
+	* GraphicsConverter.C (ConvProcess::converted): no longer receives a
+	string as first arg, reflecting change in ForkedCall interface.
+
 2002-10-18  Angus Leeming  <leeming@lyx.org>
 
 	* GraphicsCacheItem.C (findTargetFormat): add debug message.

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_);

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_;

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,50 +342,13 @@ 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();

src/support/ChangeLog

@@ -1,3 +1,11 @@
+2002-10-25  Angus Leeming  <leeming@lyx.org>
+
+	* 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  <Jean-Marc.Lasgouttes@inria.fr>
 
 	* filetools.C (getExtFromContents): remove epsi detection

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;

src/support/forkedcall.h

@@ -34,7 +34,7 @@
 #include <sigc++/signal_system.h>
 
 
-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<void, string const &, pid_t, int> SignalType;
+	typedef SigC::Signal2<void, pid_t, int> 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<SignalType> 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

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;

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<Forkedcall *> ListType;
+	typedef std::list<ForkedProcess *> ListType;
 	///
 	ListType forkedCalls;
 

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