From eb5b07f615c67fd9077fd44a6c9111ba902c4cf6 Mon Sep 17 00:00:00 2001 From: Angus Leeming Date: Thu, 31 Oct 2002 12:42:26 +0000 Subject: [PATCH] Zombie-killer. git-svn-id: svn://svn.lyx.org/lyx/lyx-devel/trunk@5563 a592a061-630c-0410-9148-cb99ea01b6c8 --- src/BufferView_pimpl.C | 4 - src/ChangeLog | 11 + src/graphics/ChangeLog | 9 + src/graphics/GraphicsConverter.C | 11 +- src/graphics/PreviewLoader.C | 32 +-- src/ispell.C | 369 +++++++++++++++++-------------- src/ispell.h | 3 + src/lyx_cb.C | 84 +++++-- src/support/ChangeLog | 8 + src/support/forkedcall.C | 151 +++++++------ src/support/forkedcall.h | 85 ++++--- src/support/forkedcontr.C | 8 +- src/support/forkedcontr.h | 6 +- 13 files changed, 468 insertions(+), 313 deletions(-) diff --git a/src/BufferView_pimpl.C b/src/BufferView_pimpl.C index 2f4848f6a2..d2fbfa2224 100644 --- a/src/BufferView_pimpl.C +++ b/src/BufferView_pimpl.C @@ -603,10 +603,6 @@ void BufferView::Pimpl::cursorToggle() return; } - /* FIXME */ - extern void reapSpellchecker(void); - reapSpellchecker(); - if (!bv_->theLockingInset()) { screen().cursorToggle(bv_); } else { diff --git a/src/ChangeLog b/src/ChangeLog index a4f8b0e2c1..881de51d80 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,14 @@ +2002-10-25 Angus Leeming + + * BufferView_pimpl.C (cursorToggle): remove reapSpellchecker hack. + + * ispell.[Ch] (setError): new method. + * ispell.C (c-tor): move out child process into new class LaunchIspell. + Use setError() insetead of goto END. + + * lyx_cb.C (AutoSave): move out child process into new class + AutoSaveBuffer. + 2002-10-30 John Levon * text3.C: make start appendix undoable diff --git a/src/graphics/ChangeLog b/src/graphics/ChangeLog index da38df4435..f47d11ac43 100644 --- a/src/graphics/ChangeLog +++ b/src/graphics/ChangeLog @@ -1,3 +1,12 @@ +2002-10-25 Angus Leeming + + * GraphicsConverter.C (Impl::converted) + * PreviewLoader.C (Impl::finishedGenerating): no longer receives a + string as first arg, reflecting change in ForkedCall interface. + + * PreviewLoader.C: use pid rather than command as identifier in + InProgress map. + 2002-10-18 Angus Leeming * GraphicsCacheItem.C (findTargetFormat): add debug message. diff --git a/src/graphics/GraphicsConverter.C b/src/graphics/GraphicsConverter.C index 7f32d30ffc..2ca7e1cacf 100644 --- a/src/graphics/GraphicsConverter.C +++ b/src/graphics/GraphicsConverter.C @@ -48,7 +48,7 @@ struct Converter::Impl : public boost::signals::trackable { * Cleans-up the temporary files, emits the finishedConversion * signal and removes the Converter from the list of all processes. */ - void converted(string const & cmd, pid_t pid, int retval); + void converted(pid_t pid, int retval); /** At the end of the conversion process inform the outside world * by emitting a signal. @@ -191,7 +191,7 @@ Converter::Impl::Impl(string const & from_file, string const & to_file_base, void Converter::Impl::startConversion() { if (!valid_process_) { - converted(string(), 0, 1); + converted(0, 1); return; } @@ -200,19 +200,18 @@ void Converter::Impl::startConversion() convert_ptr.reset(new Forkedcall::SignalType); convert_ptr->connect( - boost::bind(&Impl::converted, this, _1, _2, _3)); + boost::bind(&Impl::converted, this, _1, _2)); Forkedcall call; 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 Converter::Impl::converted(string const & /* cmd */, - pid_t /* pid */, int retval) +void Converter::Impl::converted(pid_t /* pid */, int retval) { if (finished_) // We're done already! diff --git a/src/graphics/PreviewLoader.C b/src/graphics/PreviewLoader.C index 97e59adce2..b8533eba81 100644 --- a/src/graphics/PreviewLoader.C +++ b/src/graphics/PreviewLoader.C @@ -102,12 +102,14 @@ struct InProgress { /// pid_t pid; /// + string command; + /// string metrics_file; /// BitmapFile snippets; }; -typedef map InProgressProcesses; +typedef map InProgressProcesses; typedef InProgressProcesses::value_type InProgressProcess; @@ -139,7 +141,7 @@ struct PreviewLoader::Impl : public boost::signals::trackable { private: /// Called by the Forkedcall process that generated the bitmap files. - void finishedGenerating(string const &, pid_t, int); + void finishedGenerating(pid_t, int); /// void dumpPreamble(ostream &) const; /// @@ -481,7 +483,7 @@ void PreviewLoader::Impl::startLoading() // Initiate the conversion from LaTeX to bitmap images files. Forkedcall::SignalTypePtr convert_ptr(new Forkedcall::SignalType); convert_ptr->connect( - boost::bind(&Impl::finishedGenerating, this, _1, _2, _3)); + boost::bind(&Impl::finishedGenerating, this, _1, _2)); Forkedcall call; int ret = call.startscript(command, convert_ptr); @@ -495,13 +497,22 @@ void PreviewLoader::Impl::startLoading() // Store the generation process in a list of all such processes inprogress.pid = call.pid(); - in_progress_[command] = inprogress; + inprogress.command = command; + in_progress_[inprogress.pid] = inprogress; } -void PreviewLoader::Impl::finishedGenerating(string const & command, - pid_t /* pid */, int retval) +void PreviewLoader::Impl::finishedGenerating(pid_t pid, int retval) { + // Paranoia check! + InProgressProcesses::iterator git = in_progress_.find(pid); + if (git == in_progress_.end()) { + lyxerr << "PreviewLoader::finishedGenerating(): unable to find " + "data for PID " << pid << endl; + return; + } + + string const command = git->second.command; string const status = retval > 0 ? "failed" : "succeeded"; lyxerr[Debug::GRAPHICS] << "PreviewLoader::finishedInProgress(" << retval << "): processing " << status @@ -509,15 +520,6 @@ void PreviewLoader::Impl::finishedGenerating(string const & command, if (retval > 0) return; - // Paranoia check! - InProgressProcesses::iterator git = in_progress_.find(command); - if (git == in_progress_.end()) { - lyxerr << "PreviewLoader::finishedGenerating(): unable to find " - "data for\n" - << command << "!" << endl; - return; - } - // Read the metrics file, if it exists vector ascent_fractions(git->second.snippets.size()); setAscentFractions(ascent_fractions, git->second.metrics_file); diff --git a/src/ispell.C b/src/ispell.C index e531795dbe..4159334427 100644 --- a/src/ispell.C +++ b/src/ispell.C @@ -43,7 +43,6 @@ #endif #include "LString.h" -#include "support/lstrings.h" #include "lyxrc.h" #include "language.h" #include "debug.h" @@ -51,6 +50,9 @@ #include "ispell.h" #include "WordLangTuple.h" +#include "support/forkedcall.h" +#include "support/lstrings.h" + #ifndef CXX_GLOBAL_CSTD using std::strcpy; using std::strlen; @@ -61,190 +63,246 @@ using std::strstr; using std::endl; namespace { - /// pid for the `ispell' process. - pid_t isp_pid = -1; + +/// pid for the `ispell' process. +pid_t isp_pid = -1; + +class LaunchIspell : public ForkedProcess { +public: + /// + LaunchIspell(BufferParams const & p, string const & l, + int * in, int * out) + : params(p), lang(l), pipein(in), pipeout(out) {} + /// + virtual ForkedProcess * clone() const { + return new LaunchIspell(*this); + } + /// + int start(); +private: + /// + virtual int generateChild(); + + /// + BufferParams const & params; + string const & lang; + int * const pipein; + int * const pipeout; +}; + + +int LaunchIspell::start() +{ + command_ = "ispell"; + return runNonBlocking(); +} + + +int LaunchIspell::generateChild() +{ + isp_pid = fork(); + + if (isp_pid != 0) { + // failed (-1) or parent process (>0) + return isp_pid; + } + + // child process + dup2(pipein[0], STDIN_FILENO); + dup2(pipeout[1], STDOUT_FILENO); + ::close(pipein[0]); + ::close(pipein[1]); + ::close(pipeout[0]); + ::close(pipeout[1]); + + char * argv[14]; + int argc = 0; + + char * tmp = new char[lyxrc.isp_command.length() + 1]; + lyxrc.isp_command.copy(tmp, lyxrc.isp_command.length()); + tmp[lyxrc.isp_command.length()] = '\0'; + argv[argc++] = tmp; + tmp = new char[3]; + string("-a").copy(tmp, 2); tmp[2] = '\0'; // pipe mode + argv[argc++] = tmp; + + if (lang != "default") { + tmp = new char[3]; + string("-d").copy(tmp, 2); tmp[2] = '\0'; // Dictionary file + argv[argc++] = tmp; + tmp = new char[lang.length() + 1]; + lang.copy(tmp, lang.length()); tmp[lang.length()] = '\0'; + argv[argc++] = tmp; + } + + if (lyxrc.isp_accept_compound) { + // Consider run-together words as legal compounds + tmp = new char[3]; + string("-C").copy(tmp, 2); tmp[2] = '\0'; + argv[argc++] = tmp; + } else { + // Report run-together words with + // missing blanks as errors + tmp = new char[3]; + string("-B").copy(tmp, 2); tmp[2] = '\0'; + argv[argc++] = tmp; + } + if (lyxrc.isp_use_esc_chars) { + // Specify additional characters that + // can be part of a word + tmp = new char[3]; + string("-w").copy(tmp, 2); tmp[2] = '\0'; + argv[argc++] = tmp; + // Put the escape chars in ""s + string tms = "\"" + lyxrc.isp_esc_chars + "\""; + tmp = new char[tms.length() + 1]; + tms.copy(tmp, tms.length()); tmp[tms.length()] = '\0'; + argv[argc++] = tmp; + } + if (lyxrc.isp_use_pers_dict) { + // Specify an alternate personal dictionary + tmp = new char[3]; + string("-p").copy(tmp, 2); + tmp[2]= '\0'; + argv[argc++] = tmp; + tmp = new char[lyxrc.isp_pers_dict.length() + 1]; + lyxrc.isp_pers_dict.copy(tmp, lyxrc.isp_pers_dict.length()); + tmp[lyxrc.isp_pers_dict.length()] = '\0'; + argv[argc++] = tmp; + } + if (lyxrc.isp_use_input_encoding && + params.inputenc != "default") { + string enc = (params.inputenc == "auto") + ? params.language->encoding()->LatexName() + : params.inputenc; + string::size_type n = enc.length(); + tmp = new char[3]; + string("-T").copy(tmp, 2); + tmp[2] = '\0'; + argv[argc++] = tmp; // Input encoding + tmp = new char[n + 1]; + enc.copy(tmp, n); + tmp[n] = '\0'; + argv[argc++] = tmp; + } + + argv[argc++] = 0; + + execvp(argv[0], const_cast(argv)); + + // free the memory used by string::copy in the + // setup of argv + for (int i = 0; i < argc - 1; ++i) + delete[] argv[i]; + + lyxerr << "LyX: Failed to start ispell!" << endl; + _exit(0); } +} // namespace anon + + ISpell::ISpell(BufferParams const & params, string const & lang) : str(0) { static char o_buf[BUFSIZ]; // jc: it could be smaller int pipein[2]; int pipeout[2]; - char * argv[14]; - int argc; isp_pid = -1; if (pipe(pipein) == -1 || pipe(pipeout) == -1) { lyxerr << "LyX: Can't create pipe for spellchecker!" << endl; - goto END; + setError(); + return; } if ((out = fdopen(pipein[1], "w")) == 0) { lyxerr << "LyX: Can't create stream for pipe for spellchecker!" << endl; - goto END; + setError(); + return; } if ((in = fdopen(pipeout[0], "r")) == 0) { lyxerr <<"LyX: Can't create stream for pipe for spellchecker!" << endl; - goto END; + setError(); + return; } setvbuf(out, o_buf, _IOLBF, BUFSIZ); isp_fd = pipeout[0]; - isp_pid = fork(); - + LaunchIspell childprocess(params, lang, pipein, pipeout); + isp_pid = childprocess.start(); if (isp_pid == -1) { lyxerr << "LyX: Can't create child process for spellchecker!" << endl; - goto END; + setError(); + return; } - if (isp_pid == 0) { - /* child process */ - dup2(pipein[0], STDIN_FILENO); - dup2(pipeout[1], STDOUT_FILENO); - ::close(pipein[0]); - ::close(pipein[1]); + setError(); + /* Parent process: Read ispells identification message */ + // Hmm...what are we using this id msg for? Nothing? (Lgb) + // Actually I used it to tell if it's truly Ispell or if it's + // aspell -- (kevinatk@home.com) + // But no code actually used the results for anything useful + // so I removed it again. Perhaps we can remove this code too. + // - jbl + char buf[2048]; + fd_set infds; + struct timeval tv; + int retval = 0; + FD_ZERO(&infds); + FD_SET(pipeout[0], &infds); + tv.tv_sec = 15; // fifteen second timeout. Probably too much, + // but it can't really hurt. + tv.tv_usec = 0; + + // Configure provides us with macros which are supposed to do + // the right typecast. + retval = select(SELECT_TYPE_ARG1 (pipeout[0]+1), + SELECT_TYPE_ARG234 (&infds), + 0, + 0, + SELECT_TYPE_ARG5 (&tv)); + + if (retval > 0) { + // Ok, do the reading. We don't have to FD_ISSET since + // there is only one fd in infds. + fgets(buf, 2048, in); + + fputs("!\n", out); // Set terse mode (silently accept correct words) + + } else if (retval == 0) { + // timeout. Give nice message to user. + lyxerr << "Ispell read timed out, what now?" << endl; + // This probably works but could need some thought + isp_pid = -1; ::close(pipeout[0]); ::close(pipeout[1]); + ::close(pipein[0]); + ::close(pipein[1]); + isp_fd = -1; + } else { + // Select returned error + lyxerr << "Select on ispell returned error, what now?" << endl; + } +} - argc = 0; - char * tmp = new char[lyxrc.isp_command.length() + 1]; - lyxrc.isp_command.copy(tmp, lyxrc.isp_command.length()); - tmp[lyxrc.isp_command.length()] = '\0'; - argv[argc++] = tmp; - tmp = new char[3]; - string("-a").copy(tmp, 2); tmp[2] = '\0'; // pipe mode - argv[argc++] = tmp; - if (lang != "default") { - tmp = new char[3]; - string("-d").copy(tmp, 2); tmp[2] = '\0'; // Dictionary file - argv[argc++] = tmp; - tmp = new char[lang.length() + 1]; - lang.copy(tmp, lang.length()); tmp[lang.length()] = '\0'; - argv[argc++] = tmp; - } - - if (lyxrc.isp_accept_compound) { - // Consider run-together words as legal compounds - tmp = new char[3]; - string("-C").copy(tmp, 2); tmp[2] = '\0'; - argv[argc++] = tmp; - } else { - // Report run-together words with - // missing blanks as errors - tmp = new char[3]; - string("-B").copy(tmp, 2); tmp[2] = '\0'; - argv[argc++] = tmp; - } - if (lyxrc.isp_use_esc_chars) { - // Specify additional characters that - // can be part of a word - tmp = new char[3]; - string("-w").copy(tmp, 2); tmp[2] = '\0'; - argv[argc++] = tmp; - // Put the escape chars in ""s - string tms = "\"" + lyxrc.isp_esc_chars + "\""; - tmp = new char[tms.length() + 1]; - tms.copy(tmp, tms.length()); tmp[tms.length()] = '\0'; - argv[argc++] = tmp; - } - if (lyxrc.isp_use_pers_dict) { - // Specify an alternate personal dictionary - tmp = new char[3]; - string("-p").copy(tmp, 2); - tmp[2]= '\0'; - argv[argc++] = tmp; - tmp = new char[lyxrc.isp_pers_dict.length() + 1]; - lyxrc.isp_pers_dict.copy(tmp, lyxrc.isp_pers_dict.length()); - tmp[lyxrc.isp_pers_dict.length()] = '\0'; - argv[argc++] = tmp; - } - if (lyxrc.isp_use_input_encoding && - params.inputenc != "default") { - string enc = (params.inputenc == "auto") - ? params.language->encoding()->LatexName() - : params.inputenc; - string::size_type n = enc.length(); - tmp = new char[3]; - string("-T").copy(tmp, 2); - tmp[2] = '\0'; - argv[argc++] = tmp; // Input encoding - tmp = new char[n + 1]; - enc.copy(tmp, n); - tmp[n] = '\0'; - argv[argc++] = tmp; - } - - argv[argc++] = 0; - - execvp(argv[0], const_cast(argv)); - - // free the memory used by string::copy in the - // setup of argv - for (int i = 0; i < argc - 1; ++i) - delete[] argv[i]; - - lyxerr << "LyX: Failed to start ispell!" << endl; - _exit(0); - } - { - /* Parent process: Read ispells identification message */ - // Hmm...what are we using this id msg for? Nothing? (Lgb) - // Actually I used it to tell if it's truly Ispell or if it's - // aspell -- (kevinatk@home.com) - // But no code actually used the results for anything useful - // so I removed it again. Perhaps we can remove this code too. - // - jbl - char buf[2048]; - fd_set infds; - struct timeval tv; - int retval = 0; - FD_ZERO(&infds); - FD_SET(pipeout[0], &infds); - tv.tv_sec = 15; // fifteen second timeout. Probably too much, - // but it can't really hurt. - tv.tv_usec = 0; - - // Configure provides us with macros which are supposed to do - // the right typecast. - retval = select(SELECT_TYPE_ARG1 (pipeout[0]+1), - SELECT_TYPE_ARG234 (&infds), - 0, - 0, - SELECT_TYPE_ARG5 (&tv)); - - if (retval > 0) { - // Ok, do the reading. We don't have to FD_ISSET since - // there is only one fd in infds. - fgets(buf, 2048, in); - - fputs("!\n", out); // Set terse mode (silently accept correct words) - - } else if (retval == 0) { - // timeout. Give nice message to user. - lyxerr << "Ispell read timed out, what now?" << endl; - // This probably works but could need some thought - isp_pid = -1; - ::close(pipeout[0]); - ::close(pipeout[1]); - ::close(pipein[0]); - ::close(pipein[1]); - isp_fd = -1; - } else { - // Select returned error - lyxerr << "Select on ispell returned error, what now?" << endl; - } - } - END: +ISpell::~ISpell() +{ + delete[] str; +} + + +void ISpell::setError() +{ if (isp_pid == -1) { error_ = "\n\n" @@ -260,25 +318,6 @@ ISpell::ISpell(BufferParams const & params, string const & lang) } -ISpell::~ISpell() -{ - delete[] str; -} - - -/* FIXME: this is a minimalist solution until the above - * code is able to work with forkedcall.h. We only need - * to reap the zombies here. - */ -void reapSpellchecker(void) -{ - if (isp_pid == -1) - return; - - waitpid(isp_pid, 0, WNOHANG); -} - - string const ISpell::nextMiss() { if (str == 0 || *(e+1) == '\0') diff --git a/src/ispell.h b/src/ispell.h index 2e89f00fbb..c0917c2e51 100644 --- a/src/ispell.h +++ b/src/ispell.h @@ -48,6 +48,9 @@ public: virtual string const error(); private: + /// + void setError(); + /// instream to communicate with ispell FILE * in; /// outstream to communicate with ispell diff --git a/src/lyx_cb.C b/src/lyx_cb.C index 30f2d89da1..040f63f29d 100644 --- a/src/lyx_cb.C +++ b/src/lyx_cb.C @@ -31,6 +31,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" @@ -243,28 +244,38 @@ void QuitLyX() } +namespace { -void AutoSave(BufferView * bv) - // should probably be moved into BufferList (Lgb) - // Perfect target for a thread... -{ - if (!bv->available()) - return; - - if (bv->buffer()->isBakClean() || bv->buffer()->isReadonly()) { - // We don't save now, but we'll try again later - bv->owner()->resetAutosaveTimer(); - return; +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(); +} - bv->owner()->message(_("Autosaving current document...")); - - // create autosave filename - string fname = bv->buffer()->filePath(); - fname += "#"; - fname += OnlyFilename(bv->buffer()->fileName()); - fname += "#"; +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 @@ -278,9 +289,9 @@ void AutoSave(BufferView * bv) string const tmp_ret = lyx::tempName(string(), "lyxauto"); if (!tmp_ret.empty()) { - bv->buffer()->writeFile(tmp_ret); + bv_.buffer()->writeFile(tmp_ret); // assume successful write of tmp_ret - if (!lyx::rename(tmp_ret, fname)) { + if (!lyx::rename(tmp_ret, fname_)) { failed = true; // most likely couldn't move between filesystems // unless write of tmp_ret failed @@ -293,18 +304,47 @@ void AutoSave(BufferView * bv) if (failed) { // failed to write/rename tmp_ret so try writing direct - if (!bv->buffer()->writeFile(fname)) { + if (!bv_.buffer()->writeFile(fname_)) { // It is dangerous to do this in the child, // but safe in the parent, so... if (pid == -1) - bv->owner()->message(_("Autosave failed!")); + 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) + // Perfect target for a thread... +{ + if (!bv->available()) + return; + + if (bv->buffer()->isBakClean() || bv->buffer()->isReadonly()) { + // We don't save now, but we'll try again later + bv->owner()->resetAutosaveTimer(); + return; + } + + bv->owner()->message(_("Autosaving current document...")); + + // create autosave filename + string fname = bv->buffer()->filePath(); + fname += "#"; + fname += OnlyFilename(bv->buffer()->fileName()); + fname += "#"; + AutoSaveBuffer autosave(*bv, fname); + autosave.start(); + bv->buffer()->markBakClean(); bv->owner()->resetAutosaveTimer(); } diff --git a/src/support/ChangeLog b/src/support/ChangeLog index a8b4817fb8..e71019ca2c 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-09-25 Angus Leeming * LIstream.h: diff --git a/src/support/forkedcall.C b/src/support/forkedcall.C index 5a704dce4b..6eadbc4c08 100644 --- a/src/support/forkedcall.C +++ b/src/support/forkedcall.C @@ -53,62 +53,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_->operator()(command_, pid_, retval_); - } -} - - namespace { class Murder : public boost::signals::trackable { @@ -157,9 +101,55 @@ private: } // namespace anon -void Forkedcall::kill(int tol) +ForkedProcess::ForkedProcess() + : pid_(0), retval_(0) +{} + + +void ForkedProcess::emitSignal() +{ + if (signal_.get()) { + signal_->operator()(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() { - lyxerr << "Forkedcall::kill(" << tol << ")" << std::endl; + 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; @@ -184,7 +174,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; @@ -219,8 +210,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; @@ -254,7 +267,8 @@ pid_t Forkedcall::generateChild() #ifndef __EMX__ pid_t cpid = ::fork(); - if (cpid == 0) { // child + if (cpid == 0) { + // Child execvp(syscmd, argv); // If something goes wrong, we end up here string args; @@ -263,15 +277,24 @@ pid_t Forkedcall::generateChild() args += string(" ") + argv[i++]; lyxerr << "execvp of \"" << syscmd << args << "\" failed: " << strerror(errno) << endl; + _exit(1); } #else pid_t cpid = spawnvp(P_SESSION|P_DEFAULT|P_MINIMIZE|P_BACKGROUND, syscmd, argv); #endif - if (cpid < 0) { // error - lyxerr << "Could not fork: " - << strerror(errno) << endl; + if (cpid < 0) { + // Error. + lyxerr << "Could not fork: " << strerror(errno) << endl; + } + + // Clean-up. + delete [] syscmd; + for (int i = 0; i < MAX_ARGV; ++i) { + if (argv[i] == 0) + break; + delete [] argv[i]; } return cpid; diff --git a/src/support/forkedcall.h b/src/support/forkedcall.h index 2843261ef4..6b58e14997 100644 --- a/src/support/forkedcall.h +++ b/src/support/forkedcall.h @@ -33,11 +33,12 @@ #include "LString.h" #include -#include +#include +#include #include -class Forkedcall { +class ForkedProcess { public: /// enum Starttype { @@ -48,25 +49,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. * @@ -74,7 +64,7 @@ public: * we can return easily to C++ methods, rather than just globally * accessible functions. */ - typedef boost::signal3 SignalType; + typedef boost::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 @@ -86,9 +76,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! */ @@ -108,6 +95,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" @@ -115,14 +105,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 runBlocking(); + /** Do not wait for child process to finish. + * Returns returncode from child. + */ + int runNonBlocking(); + /// Callback function SignalTypePtr signal_; - /// Commmand line + /// identifying command (for display in the GUI perhaps). string command_; /// Process ID of child @@ -130,12 +127,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 execvp() 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 0da94ef5d0..d5f5d3eacf 100644 --- a/src/support/forkedcontr.C +++ b/src/support/forkedcontr.C @@ -71,14 +71,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(); } @@ -91,7 +89,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 7493a25910..6dbed1c7ac 100644 --- a/src/support/forkedcontr.h +++ b/src/support/forkedcontr.h @@ -30,7 +30,7 @@ #pragma interface #endif -class Forkedcall; +class ForkedProcess; class Timeout; class ForkedcallsController : public boost::signals::trackable { @@ -46,7 +46,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 @@ -75,7 +75,7 @@ private: ForkedcallsController(ForkedcallsController const &); /// The child processes - typedef std::list ListType; + typedef std::list ListType; /// ListType forkedCalls; -- 2.39.2