From 042d5a024251a9eae85814b2d5fa84eca4bf62dc Mon Sep 17 00:00:00 2001 From: Georg Baum Date: Wed, 7 Jun 2006 20:08:37 +0000 Subject: [PATCH] Fix crash on exit (bug 2549) by correct usage of QApplication * src/lyx_cb.C (quitLyX): lyx_gui::exit takes now an argument * src/frontends/{gtk,xforms}/lyx_gui.C (lyx_gui::parse_init): rename to lyx_gui::exec and call LyX::exec2 (void lyx_gui::exit): add exit status argument * src/frontends/qt{3,4}/lyx_gui.C (cleanup): new function for pointer cleanup (lyx_gui::parse_init): rename to lyx_gui::exec and call LyX::exec2, turn static variables into automatic variables (void lyx_gui::exit): add exit status argument (start): Use cleanup() (exit): ditto * src/frontends/lyx_gui.h (parse_init): remove (exec): new (exit): Take exist status argument * src/lyx_main.[Ch] (LyX::priv_exec): split into LyX::priv_exec and LyX::exec2 * src/lyx_main.C (lyx_exit): New, choose the right exit function (showFileError): call lyx_exit (LyX::queryUserLyXDir): ditto (LyX::init): ditto (LyX::priv_exec): ditto (LyX::priv_exec): Replace want_gui by lyx_gui::use_gui (LyX::priv_exec): replace lyx_gui::parse_init by lyx_gui::exec and exec2 (LyX::init): Replace gui argument by lyx_gui::use_gui git-svn-id: svn://svn.lyx.org/lyx/lyx-devel/trunk@14036 a592a061-630c-0410-9148-cb99ea01b6c8 --- src/frontends/gtk/lyx_gui.C | 7 +++- src/frontends/lyx_gui.h | 12 +++--- src/frontends/qt3/lyx_gui.C | 48 +++++++++++++---------- src/frontends/qt4/lyx_gui.C | 72 ++++++++++++---------------------- src/frontends/xforms/lyx_gui.C | 9 ++++- src/lyx_cb.C | 2 +- src/lyx_main.C | 59 +++++++++++++++++----------- src/lyx_main.h | 16 +++++++- 8 files changed, 123 insertions(+), 102 deletions(-) diff --git a/src/frontends/gtk/lyx_gui.C b/src/frontends/gtk/lyx_gui.C index 4389e6dbcf..c993dd4fbc 100644 --- a/src/frontends/gtk/lyx_gui.C +++ b/src/frontends/gtk/lyx_gui.C @@ -100,7 +100,7 @@ int getDPI() } // namespace anon -void lyx_gui::parse_init(int & argc, char * argv[]) +void lyx_gui::exec(int & argc, char * argv[]) { new Gtk::Main(argc, argv); @@ -112,6 +112,8 @@ void lyx_gui::parse_init(int & argc, char * argv[]) // must do this /before/ lyxrc gets read lyxrc.dpi = getDPI(); + + LyX::ref().exec2(argc, argv); } @@ -152,8 +154,9 @@ void lyx_gui::start(string const & batch, std::vector const & files, } -void lyx_gui::exit() +void lyx_gui::exit(int /*status*/) { + // FIXME: Don't ignore status Gtk::Main::quit(); } diff --git a/src/frontends/lyx_gui.h b/src/frontends/lyx_gui.h index 8a53f66cda..e11fb99040 100644 --- a/src/frontends/lyx_gui.h +++ b/src/frontends/lyx_gui.h @@ -46,9 +46,6 @@ std::string const sans_font_name(); /// return a suitable monospaced font name (called from non-gui context too !) std::string const typewriter_font_name(); -/// parse command line and do basic initialisation -void parse_init(int & argc, char * argv[]); - /** * set up GUI parameters. At this point lyxrc may * be used. @@ -60,7 +57,12 @@ void parse_lyxrc(); * batch commands, and loading the given documents */ void start(std::string const & batch, std::vector const & files, - unsigned int width, unsigned int height, int posx, int posy); + unsigned int width, unsigned int height, int posx, int posy); + +/** + * Enter the main event loop (\sa LyX::exec2) + */ +void exec(int & argc, char * argv[]); /** * Synchronise all pending events. @@ -70,7 +72,7 @@ void sync_events(); /** * quit running LyX */ -void exit(); +void exit(int); /** * return the status flag for a given action. This can be used to tell diff --git a/src/frontends/qt3/lyx_gui.C b/src/frontends/qt3/lyx_gui.C index f4d5a123ac..a9cd42ef72 100644 --- a/src/frontends/qt3/lyx_gui.C +++ b/src/frontends/qt3/lyx_gui.C @@ -79,6 +79,10 @@ using std::string; extern BufferList bufferlist; +// FIXME: wrong place ! +LyXServer * lyxserver; +LyXServerSocket * lyxsocket; + namespace { int getDPI() @@ -90,11 +94,15 @@ int getDPI() map > socket_callbacks; -} // namespace anon +void cleanup() +{ + delete lyxsocket; + lyxsocket = 0; + delete lyxserver; + lyxserver = 0; +} -// FIXME: wrong place ! -LyXServer * lyxserver; -LyXServerSocket * lyxsocket; +} // namespace anon // in QLyXKeySym.C extern void initEncodings(); @@ -104,7 +112,6 @@ extern bool lyxX11EventFilter(XEvent * xev); #endif #ifdef Q_WS_MACX -extern bool macEventFilter(EventRef event); extern pascal OSErr handleOpenDocuments(const AppleEvent* inEvent, AppleEvent* /*reply*/, long /*refCon*/); @@ -153,22 +160,23 @@ namespace lyx_gui { bool use_gui = true; -void parse_init(int & argc, char * argv[]) + +void exec(int & argc, char * argv[]) { // Force adding of font path _before_ QApplication is initialized FontLoader::initFontPath(); - static LQApplication app(argc, argv); + LQApplication app(argc, argv); #if QT_VERSION >= 0x030200 // install translation file for Qt built-in dialogs // These are only installed since Qt 3.2.x - static QTranslator qt_trans(0); + QTranslator qt_trans(0); if (qt_trans.load(QString("qt_") + QTextCodec::locale(), qInstallPathTranslations())) { - app.installTranslator(&qt_trans); + qApp->installTranslator(&qt_trans); // even if the language calls for RtL, don't do that - app.setReverseLayout(false); + qApp->setReverseLayout(false); lyxerr[Debug::GUI] << "Successfully installed Qt translations for locale " << QTextCodec::locale() << std::endl; @@ -182,7 +190,7 @@ void parse_init(int & argc, char * argv[]) // These translations are meant to break Qt/Mac menu merging // algorithm on some entries. It lists the menu names that // should not be moved to the LyX menu - static QTranslator aqua_trans(0); + QTranslator aqua_trans(0); aqua_trans.insert(QTranslatorMessage("QMenuBar", "Setting", 0, "do_not_merge_me")); aqua_trans.insert(QTranslatorMessage("QMenuBar", "Config", 0, @@ -192,7 +200,7 @@ void parse_init(int & argc, char * argv[]) aqua_trans.insert(QTranslatorMessage("QMenuBar", "Setup", 0, "do_not_merge_me")); - app.installTranslator(&aqua_trans); + qApp->installTranslator(&aqua_trans); #endif using namespace lyx::graphics; @@ -204,6 +212,8 @@ void parse_init(int & argc, char * argv[]) lyxrc.dpi = getDPI(); LoaderQueue::setPriority(10,100); + + LyX::ref().exec2(argc, argv); } @@ -245,9 +255,7 @@ void start(string const & batch, vector const & files, qApp->exec(); // FIXME - delete lyxsocket; - delete lyxserver; - lyxserver = 0; + cleanup(); } @@ -263,18 +271,16 @@ void sync_events() } -void exit() +void exit(int status) { - delete lyxsocket; - delete lyxserver; - lyxserver = 0; + cleanup(); - // we cannot call qApp->exit(0) - that could return us + // we cannot call QApplication::exit(status) - that could return us // into a static dialog return in the lyx code (for example, // load autosave file QMessageBox. We have to just get the hell // out. - ::exit(0); + ::exit(status); } diff --git a/src/frontends/qt4/lyx_gui.C b/src/frontends/qt4/lyx_gui.C index fe523c964c..f0297488e7 100644 --- a/src/frontends/qt4/lyx_gui.C +++ b/src/frontends/qt4/lyx_gui.C @@ -76,6 +76,10 @@ using std::string; extern BufferList bufferlist; +// FIXME: wrong place ! +LyXServer * lyxserver; +LyXServerSocket * lyxsocket; + namespace { int getDPI() @@ -86,11 +90,15 @@ int getDPI() map > socket_callbacks; -} // namespace anon +void cleanup() +{ + delete lyxsocket; + lyxsocket = 0; + delete lyxserver; + lyxserver = 0; +} -// FIXME: wrong place ! -LyXServer * lyxserver; -LyXServerSocket * lyxsocket; +} // namespace anon // in QLyXKeySym.C extern void initEncodings(); @@ -145,50 +153,21 @@ bool LQApplication::macEventFilter(EventRef event) #endif -namespace { - -LQApplication * app = 0; - -} - - namespace lyx_gui { bool use_gui = true; -void parse_init(int & argc, char * argv[]) + +void exec(int & argc, char * argv[]) { - /* - FIXME : Abdel 29/05/2006 (younes.a@free.fr) - reorganize this code. In particular make sure that this - advise from Qt documentation is respected: - - Since the QApplication object does so much initialization, it - must be created before any other objects related to the user - interface are created. - - Right now this is not the case. For example, the call to - "FontLoader::initFontPath()" below is doned before the QApplication - creation. Moreover, I suspect that a number of global variables - contains Qt object that are initialized before the passage through - parse_init(). This might also explain the message displayed by Qt - that caused the hanging: - - QObject::killTimer: timers cannot be stopped from another thread - */ - // Force adding of font path _before_ QApplication is initialized FontLoader::initFontPath(); -#ifdef Q_WS_WIN - static QApplication win_app(argc, argv); -#else - app = new LQApplication(argc, argv); -#endif + LQApplication app(argc, argv); // install translation file for Qt built-in dialogs // These are only installed since Qt 3.2.x - static QTranslator qt_trans(0); + QTranslator qt_trans(0); if (qt_trans.load(QString("qt_") + QTextCodec::locale(), qInstallPathTranslations())) { qApp->installTranslator(&qt_trans); @@ -206,7 +185,7 @@ void parse_init(int & argc, char * argv[]) // These translations are meant to break Qt/Mac menu merging // algorithm on some entries. It lists the menu names that // should not be moved to the LyX menu - static QTranslator aqua_trans(0); + QTranslator aqua_trans(0); aqua_trans.insert(QTranslatorMessage("QMenuBar", "Setting", 0, "do_not_merge_me")); aqua_trans.insert(QTranslatorMessage("QMenuBar", "Config", 0, @@ -228,6 +207,8 @@ void parse_init(int & argc, char * argv[]) lyxrc.dpi = getDPI(); LoaderQueue::setPriority(10,100); + + LyX::ref().exec2(argc, argv); } @@ -269,10 +250,7 @@ void start(string const & batch, vector const & files, qApp->exec(); // FIXME - delete lyxsocket; - delete lyxserver; - lyxserver = 0; - delete app; + cleanup(); } @@ -288,18 +266,16 @@ void sync_events() } -void exit() +void exit(int status) { - delete lyxsocket; - delete lyxserver; - lyxserver = 0; + cleanup(); - // we cannot call qApp->exit(0) - that could return us + // we cannot call QApplication::exit(status) - that could return us // into a static dialog return in the lyx code (for example, // load autosave file QMessageBox. We have to just get the hell // out. - ::exit(0); + ::exit(status); } diff --git a/src/frontends/xforms/lyx_gui.C b/src/frontends/xforms/lyx_gui.C index 6e633f11b6..cbabd6d28d 100644 --- a/src/frontends/xforms/lyx_gui.C +++ b/src/frontends/xforms/lyx_gui.C @@ -83,6 +83,7 @@ namespace { /// quit lyx bool finished = false; +int exit_status = 0; /// estimate DPI from X server int getDPI() @@ -152,7 +153,7 @@ namespace lyx_gui { bool use_gui = true; -void parse_init(int & argc, char * argv[]) +void exec(int & argc, char * argv[]) { setDefaults(); @@ -197,6 +198,8 @@ void parse_init(int & argc, char * argv[]) lyxrc.dpi = getDPI(); LoaderQueue::setPriority(10,100); + + LyX::ref().exec2(argc, argv); } @@ -313,12 +316,14 @@ void start(string const & batch, vector const & files, // FIXME: breaks emergencyCleanup delete lyxsocket; delete lyxserver; + ::exit(exit_status); } -void exit() +void exit(int status) { finished = true; + exit_status = status; } diff --git a/src/lyx_cb.C b/src/lyx_cb.C index a81934ee50..fcd809bec7 100644 --- a/src/lyx_cb.C +++ b/src/lyx_cb.C @@ -216,7 +216,7 @@ void quitLyX(bool noask) Alert::warning(_("Unable to remove temporary directory"), msg); } - lyx_gui::exit(); + lyx_gui::exit(0); } diff --git a/src/lyx_main.C b/src/lyx_main.C index ecea886fc3..8df387930f 100644 --- a/src/lyx_main.C +++ b/src/lyx_main.C @@ -107,12 +107,23 @@ string cl_system_support; string cl_user_support; +void lyx_exit(int status) +{ + // FIXME: We should not directly call exit(), since it only + // guarantees a return to the system, no application cleanup. + // This may cause troubles with not executed destructors. + if (lyx_gui::use_gui) + lyx_gui::exit(status); + exit(status); +} + + void showFileError(string const & error) { Alert::warning(_("Could not read configuration file"), bformat(_("Error while reading the configuration file\n%1$s.\n" "Please check your installation."), error)); - exit(EXIT_FAILURE); + lyx_exit(EXIT_FAILURE); } @@ -204,14 +215,21 @@ void LyX::priv_exec(int & argc, char * argv[]) { // Here we need to parse the command line. At least // we need to parse for "-dbg" and "-help" - bool const want_gui = easyParse(argc, argv); + lyx_gui::use_gui = easyParse(argc, argv); lyx::support::init_package(argv[0], cl_system_support, cl_user_support, lyx::support::top_build_dir_is_one_level_up); - if (want_gui) - lyx_gui::parse_init(argc, argv); + // Start the real execution loop. + if (lyx_gui::use_gui) + lyx_gui::exec(argc, argv); + else + exec2(argc, argv); +} + +void LyX::exec2(int & argc, char * argv[]) +{ // check for any spurious extra arguments // other than documents for (int argi = 1; argi < argc ; ++argi) { @@ -224,10 +242,10 @@ void LyX::priv_exec(int & argc, char * argv[]) // Initialization of LyX (reads lyxrc and more) lyxerr[Debug::INIT] << "Initializing LyX::init..." << endl; - init(want_gui); + init(); lyxerr[Debug::INIT] << "Initializing LyX::init...done" << endl; - if (want_gui) + if (lyx_gui::use_gui) lyx_gui::parse_lyxrc(); vector files; @@ -278,13 +296,13 @@ void LyX::priv_exec(int & argc, char * argv[]) bool success = false; if (last_loaded->dispatch(batch_command, &success)) { quitLyX(false); - exit(!success); + lyx_exit(!success); } } files.clear(); // the files are already loaded } - if (want_gui) { + if (lyx_gui::use_gui) { // determine windows size and position, from lyxrc and/or session // initial geometry unsigned int width = 690; @@ -296,10 +314,10 @@ void LyX::priv_exec(int & argc, char * argv[]) } // if lyxrc returns (0,0), then use session info else { - string val = LyX::ref().session().loadSessionInfo("WindowWidth"); + string val = session().loadSessionInfo("WindowWidth"); if (!val.empty()) width = convert(val); - val = LyX::ref().session().loadSessionInfo("WindowHeight"); + val = session().loadSessionInfo("WindowHeight"); if (!val.empty()) height = convert(val); } @@ -307,10 +325,10 @@ void LyX::priv_exec(int & argc, char * argv[]) int posx = -1; int posy = -1; if (lyxrc.geometry_xysaved) { - string val = LyX::ref().session().loadSessionInfo("WindowPosX"); + string val = session().loadSessionInfo("WindowPosX"); if (!val.empty()) posx = convert(val); - val = LyX::ref().session().loadSessionInfo("WindowPosY"); + val = session().loadSessionInfo("WindowPosY"); if (!val.empty()) posy = convert(val); } @@ -318,7 +336,7 @@ void LyX::priv_exec(int & argc, char * argv[]) } else { // Something went wrong above quitLyX(false); - exit(EXIT_FAILURE); + lyx_exit(EXIT_FAILURE); } } @@ -430,7 +448,7 @@ void LyX::printError(ErrorItem const & ei) } -void LyX::init(bool gui) +void LyX::init() { #ifdef SIGHUP signal(SIGHUP, error_handler); @@ -441,9 +459,6 @@ void LyX::init(bool gui) signal(SIGTERM, error_handler); // SIGPIPE can be safely ignored. - // Disable gui when easyparse says so - lyx_gui::use_gui = gui; - lyxrc.tempdir_path = package().temp_dir(); lyxrc.document_path = package().document_dir(); @@ -478,7 +493,7 @@ void LyX::init(bool gui) // Check that user LyX directory is ok. We don't do that if // running in batch mode. - if (gui) { + if (lyx_gui::use_gui) { if (queryUserLyXDir(package().explicit_user_support())) reconfigureUserLyXDir(); } else { @@ -507,7 +522,7 @@ void LyX::init(bool gui) lyxerr[Debug::INIT] << "Reading layouts..." << endl; LyXSetStyle(); - if (gui) { + if (lyx_gui::use_gui) { // Set up bindings toplevel_keymap.reset(new kb_keymap); defaultKeyBindings(toplevel_keymap.get()); @@ -540,7 +555,7 @@ void LyX::init(bool gui) // close to zero. We therefore don't try to overcome this // problem with e.g. asking the user for a new path and // trying again but simply exit. - exit(EXIT_FAILURE); + lyx_exit(EXIT_FAILURE); } if (lyxerr.debugging(Debug::INIT)) { @@ -688,7 +703,7 @@ bool LyX::queryUserLyXDir(bool explicit_userdir) _("&Create directory"), _("&Exit LyX"))) { lyxerr << _("No user LyX directory. Exiting.") << endl; - exit(1); + lyx_exit(EXIT_FAILURE); } lyxerr << bformat(_("LyX: Creating directory %1$s"), @@ -699,7 +714,7 @@ bool LyX::queryUserLyXDir(bool explicit_userdir) // Failed, so let's exit. lyxerr << _("Failed to create directory. Exiting.") << endl; - exit(1); + lyx_exit(EXIT_FAILURE); } return true; diff --git a/src/lyx_main.h b/src/lyx_main.h index 426a7810ee..15bb2ef636 100644 --- a/src/lyx_main.h +++ b/src/lyx_main.h @@ -34,7 +34,21 @@ namespace lyx { /// initial startup class LyX : boost::noncopyable { public: + /** + * Execute LyX. The startup sequence is as follows: + * -# LyX::exec() + * -# LyX::priv_exec() + * -# lyx_gui::exec() + * -# LyX::exec2() + * Step 3 is omitted if no gui is wanted. We need lyx_gui::exec() + * only to create the QApplication object in the qt frontend. All + * attempts with static and dynamically allocated QApplication + * objects lead either to harmless error messages on exit + * ("Mutex destroy failure") or crashes (OS X). + */ static void exec(int & argc, char * argv[]); + /// Execute LyX (inner execution loop, \sa exec) + void exec2(int & argc, char * argv[]); static LyX & ref(); static LyX const & cref(); @@ -58,7 +72,7 @@ private: void priv_exec(int & argc, char * argv[]); /// initial LyX set up - void init(bool); + void init(); /// set up the default key bindings void defaultKeyBindings(kb_keymap * kbmap); /// set up the default dead key bindings if requested