Mercurial > octave
changeset 28347:00a9a49c7670 stable
improve interpreter shutdown process (bug #56952)
This change is a further attempt to avoid segfaults when shutting down
the interpreter and exiting the GUI event loop. The latest approach
is to have the interpreter signal that it is finished with "normal"
command execution (REPL, command line script, or --eval option code),
then let the GUI thread process any remaining functions in its event
loop(s) then signal back to the interpreter that it is OK to shutdown.
Once the shutdown has happened (which may involve further calls to the
GUI thread while executing atexit functions or finish.m or other
shutdown code, the interpreter signals back to the GUI that shutdown
is complete. At that point, the GUI can delete the interpreter object
and exit.
* ObjectProxy.h, ObjectProxy.cc (ObjectProxy::sendFinalize): New signal.
(ObjectProxy::ObjectProxy): Connect/disconnect sendFinalize signal.
(ObjectProxy::update, ObjectProxy::finalize): Use normal signal/slot
connection.
* interpreter-qobject.h, interpreter-qobject.cc
(interpreter_qobject::ready): Rename from octave_ready_signal. Change
all uses.
(interpreter_qobject::execution_finished): Rename from
octave_finished_singal. Change all uses.
(interpreter_qobject::shutdown_finished): New signal.
(interpreter_qobject::shutdown): New slot.
(interpreter_qobject::execute): After interpreter finishes with normal
execution, simply signal that execution has finished. Don't attempt
to disable connecton to GUI or cleanup/delete interpreter.
* octave-qobject.h, octave-qobject.cc
(base_qobject::handle_interpreter_execution_finished): Rename from
handle_octave_finished): Simply emit a signal requesting an orderly
shutdown of the interpreter. Change all uses.
(base_qobject::handle_interpreter_shutdown_finished): New slot.
(base_qobject::request_interpreter_shutdown): New signal.
(base_qobject::base_qobject): Connect request_interpreter_shutdown to
interpreter_qobject::shutdown slot.
* interpreter.h, interpreter.cc (interpreter::shutdown): Rename from
cleanup and make public.
(interpreter::~interpreter): Don't call cleanup here.
| author | John W. Eaton <jwe@octave.org> |
|---|---|
| date | Fri, 22 May 2020 12:54:13 -0400 |
| parents | d0555f415774 |
| children | d73e4749d8dd 8513a18439ca |
| files | libgui/graphics/ObjectProxy.cc libgui/graphics/ObjectProxy.h libgui/src/interpreter-qobject.cc libgui/src/interpreter-qobject.h libgui/src/octave-qobject.cc libgui/src/octave-qobject.h libinterp/corefcn/interpreter.cc libinterp/corefcn/interpreter.h |
| diffstat | 8 files changed, 167 insertions(+), 139 deletions(-) [+] |
line wrap: on
line diff
--- a/libgui/graphics/ObjectProxy.cc Thu May 21 11:23:24 2020 -0400 +++ b/libgui/graphics/ObjectProxy.cc Fri May 22 12:54:13 2020 -0400 @@ -55,6 +55,8 @@ { disconnect (this, SIGNAL (sendUpdate (int)), m_object, SLOT (slotUpdate (int))); + disconnect (this, SIGNAL (sendFinalize (void)), + m_object, SLOT (slotFinalize (void))); disconnect (this, SIGNAL (sendRedraw (void)), m_object, SLOT (slotRedraw (void))); disconnect (this, SIGNAL (sendShow (void)), @@ -69,6 +71,8 @@ { connect (this, SIGNAL (sendUpdate (int)), m_object, SLOT (slotUpdate (int))); + connect (this, SIGNAL (sendFinalize (void)), + m_object, SLOT (slotFinalize (void))); connect (this, SIGNAL (sendRedraw (void)), m_object, SLOT (slotRedraw (void))); connect (this, SIGNAL (sendShow (void)), @@ -90,24 +94,13 @@ void ObjectProxy::update (int pId) { - if (octave::thread::is_thread ()) - emit sendUpdate (pId); - else if (m_object) - m_object->slotUpdate (pId); + emit sendUpdate (pId); } void ObjectProxy::finalize (void) { - if (! m_object) - return; - - Qt::ConnectionType t = Qt::BlockingQueuedConnection; - - if (QThread::currentThread () == QCoreApplication::instance ()->thread ()) - t = Qt::DirectConnection; - - QMetaObject::invokeMethod (m_object, "slotFinalize", t); + emit sendFinalize (); } void
--- a/libgui/graphics/ObjectProxy.h Thu May 21 11:23:24 2020 -0400 +++ b/libgui/graphics/ObjectProxy.h Fri May 22 12:54:13 2020 -0400 @@ -56,6 +56,7 @@ signals: void sendUpdate (int pId); + void sendFinalize (void); void sendRedraw (void); void sendShow (void); void sendPrint (const QString& file_cmd, const QString& term);
--- a/libgui/src/interpreter-qobject.cc Thu May 21 11:23:24 2020 -0400 +++ b/libgui/src/interpreter-qobject.cc Fri May 22 12:54:13 2020 -0400 @@ -78,7 +78,7 @@ m_interpreter = &interp; - emit octave_ready_signal (); + emit ready (); graphics_init (interp, m_octave_qobj); @@ -92,16 +92,31 @@ exit_status = ex.exit_status (); } - // Disable events from being passed from the GUI to the interpreter. + // Signal that the interpreter is done executing code in the main + // REPL, from script files, or command line eval arguments. By + // using a signal here, we give the GUI a chance to process any + // pending events, then signal that it is safe to shutdown the + // interpreter. Our notification here allows the GUI to insert the + // request to shutdown the interpreter in the event queue after any + // other pending signals. The application context owns the + // interpreter and will be responsible for deleting it later, when + // the application object destructor is executed. - m_interpreter = nullptr; + emit execution_finished (exit_status); + } - // Whether or not initialization succeeds we need to clean up the - // interpreter once we are done with it. + // This function is expected to be executed when the GUI signals that + // it is finished processing events and ready for the interpreter to + // perform shutdown actions. - app_context.delete_interpreter (); + void interpreter_qobject::shutdown (int exit_status) + { + if (m_interpreter) + m_interpreter->shutdown (); - emit octave_finished_signal (exit_status); + // Signal that the interpreter has executed shutdown actions. + + emit shutdown_finished (exit_status); } void interpreter_qobject::interpreter_event (const fcn_callback& fcn)
--- a/libgui/src/interpreter-qobject.h Thu May 21 11:23:24 2020 -0400 +++ b/libgui/src/interpreter-qobject.h Fri May 22 12:54:13 2020 -0400 @@ -54,9 +54,11 @@ signals: - void octave_ready_signal (void); + void ready (void); - void octave_finished_signal (int); + void execution_finished (int); + + void shutdown_finished (int); public slots: @@ -82,6 +84,8 @@ void execute (void); + void shutdown (int); + private: base_qobject& m_octave_qobj;
--- a/libgui/src/octave-qobject.cc Thu May 21 11:23:24 2020 -0400 +++ b/libgui/src/octave-qobject.cc Fri May 22 12:54:13 2020 -0400 @@ -213,8 +213,14 @@ // Force left-to-right alignment (see bug #46204) m_qapplication->setLayoutDirection (Qt::LeftToRight); - connect (m_interpreter_qobj, SIGNAL (octave_finished_signal (int)), - this, SLOT (handle_octave_finished (int))); + connect (m_interpreter_qobj, SIGNAL (execution_finished (int)), + this, SLOT (handle_interpreter_execution_finished (int))); + + connect (this, SIGNAL (request_interpreter_shutdown (int)), + m_interpreter_qobj, SLOT (shutdown (int))); + + connect (m_interpreter_qobj, SIGNAL (shutdown_finished (int)), + this, SLOT (handle_interpreter_shutdown_finished (int))); connect (m_main_thread, SIGNAL (finished (void)), m_main_thread, SLOT (deleteLater (void))); @@ -283,7 +289,12 @@ return true; } - void base_qobject::handle_octave_finished (int exit_status) + void base_qobject::handle_interpreter_execution_finished (int exit_status) + { + emit request_interpreter_shutdown (exit_status); + } + + void base_qobject::handle_interpreter_shutdown_finished (int exit_status) { #if defined (Q_OS_MAC) // fprintf to stderr is needed by macOS, for poorly-understood reasons. @@ -355,7 +366,7 @@ gui_qobject::gui_qobject (qt_application& app_context) : base_qobject (app_context), m_main_window (new main_window (*this)) { - connect (m_interpreter_qobj, SIGNAL (octave_ready_signal (void)), + connect (m_interpreter_qobj, SIGNAL (ready (void)), m_main_window, SLOT (handle_octave_ready (void))); connect (qt_link (),
--- a/libgui/src/octave-qobject.h Thu May 21 11:23:24 2020 -0400 +++ b/libgui/src/octave-qobject.h Fri May 22 12:54:13 2020 -0400 @@ -127,9 +127,15 @@ virtual bool confirm_shutdown (void); + signals: + + void request_interpreter_shutdown (int); + public slots: - void handle_octave_finished (int); + void handle_interpreter_execution_finished (int); + + void handle_interpreter_shutdown_finished (int); void interpreter_event (const fcn_callback& fcn);
--- a/libinterp/corefcn/interpreter.cc Thu May 21 11:23:24 2020 -0400 +++ b/libinterp/corefcn/interpreter.cc Fri May 22 12:54:13 2020 -0400 @@ -635,8 +635,6 @@ interpreter::~interpreter (void) { - cleanup (); - delete m_gh_manager; } @@ -807,6 +805,114 @@ return exit_status; } + // Call a function with exceptions handled to avoid problems with + // errors while shutting down. + +#define OCTAVE_IGNORE_EXCEPTION(E) \ + catch (E) \ + { \ + recover_from_exception (); \ + \ + std::cerr << "error: ignoring " #E " while preparing to exit" \ + << std::endl; \ + } + +#define OCTAVE_SAFE_CALL(F, ARGS) \ + do \ + { \ + try \ + { \ + unwind_protect frame; \ + \ + frame.add_method (m_error_system, \ + &error_system::set_debug_on_error, \ + m_error_system.debug_on_error ()); \ + frame.add_method (m_error_system, \ + &error_system::set_debug_on_warning, \ + m_error_system.debug_on_warning ()); \ + \ + m_error_system.debug_on_error (false); \ + m_error_system.debug_on_warning (false); \ + \ + F ARGS; \ + } \ + OCTAVE_IGNORE_EXCEPTION (const exit_exception&) \ + OCTAVE_IGNORE_EXCEPTION (const interrupt_exception&) \ + OCTAVE_IGNORE_EXCEPTION (const execution_exception&) \ + OCTAVE_IGNORE_EXCEPTION (const std::bad_alloc&) \ + } \ + while (0) + + void interpreter::shutdown (void) + { + // If we are attached to a GUI, process pending events and + // disable the link. + + m_event_manager.process_events (true); + m_event_manager.disable (); + + OCTAVE_SAFE_CALL (m_input_system.clear_input_event_hooks, ()); + + // Any atexit functions added after this function call won't be + // executed. + + execute_atexit_fcns (); + + // Do this explicitly so that destructors for mex file objects + // are called, so that functions registered with mexAtExit are + // called. + OCTAVE_SAFE_CALL (m_symbol_table.clear_mex_functions, ()); + + OCTAVE_SAFE_CALL (command_editor::restore_terminal_state, ()); + + OCTAVE_SAFE_CALL (m_history_system.write_timestamp, ()); + + if (! command_history::ignoring_entries ()) + OCTAVE_SAFE_CALL (command_history::clean_up_and_save, ()); + + OCTAVE_SAFE_CALL (m_gh_manager->close_all_figures, ()); + + m_gtk_manager.unload_all_toolkits (); + + // FIXME: May still need something like this to ensure that + // destructors for class objects will run properly. Should that be + // done earlier? Before or after atexit functions are executed? + m_symbol_table.cleanup (); + + OCTAVE_SAFE_CALL (sysdep_cleanup, ()); + + OCTAVE_SAFE_CALL (flush_stdout, ()); + + // Don't call singleton_cleanup_list::cleanup until we have the + // problems with registering/unregistering types worked out. For + // example, uncomment the following line, then use the make_int + // function from the examples directory to create an integer + // object and then exit Octave. Octave should crash with a + // segfault when cleaning up the typinfo singleton. We need some + // way to force new octave_value_X types that are created in + // .oct files to be unregistered when the .oct file shared library + // is unloaded. + // + // OCTAVE_SAFE_CALL (singleton_cleanup_list::cleanup, ()); + } + + void interpreter::execute_atexit_fcns (void) + { + // Prevent atexit functions from adding new functions to the list. + m_executing_atexit = true; + + while (! m_atexit_fcns.empty ()) + { + std::string fcn = m_atexit_fcns.front (); + + m_atexit_fcns.pop_front (); + + OCTAVE_SAFE_CALL (feval, (fcn, octave_value_list (), 0)); + + OCTAVE_SAFE_CALL (flush_stdout, ()); + } + } + void interpreter::display_startup_message (void) const { bool inhibit_startup_message = false; @@ -1185,114 +1291,6 @@ return exit_status; } - // Call a function with exceptions handled to avoid problems with - // errors while shutting down. - -#define OCTAVE_IGNORE_EXCEPTION(E) \ - catch (E) \ - { \ - recover_from_exception (); \ - \ - std::cerr << "error: ignoring " #E " while preparing to exit" \ - << std::endl; \ - } - -#define OCTAVE_SAFE_CALL(F, ARGS) \ - do \ - { \ - try \ - { \ - unwind_protect frame; \ - \ - frame.add_method (m_error_system, \ - &error_system::set_debug_on_error, \ - m_error_system.debug_on_error ()); \ - frame.add_method (m_error_system, \ - &error_system::set_debug_on_warning, \ - m_error_system.debug_on_warning ()); \ - \ - m_error_system.debug_on_error (false); \ - m_error_system.debug_on_warning (false); \ - \ - F ARGS; \ - } \ - OCTAVE_IGNORE_EXCEPTION (const exit_exception&) \ - OCTAVE_IGNORE_EXCEPTION (const interrupt_exception&) \ - OCTAVE_IGNORE_EXCEPTION (const execution_exception&) \ - OCTAVE_IGNORE_EXCEPTION (const std::bad_alloc&) \ - } \ - while (0) - - void interpreter::cleanup (void) - { - // If we are attached to a GUI, process pending events and - // disable the link. - - m_event_manager.process_events (true); - m_event_manager.disable (); - - OCTAVE_SAFE_CALL (m_input_system.clear_input_event_hooks, ()); - - // Any atexit functions added after this function call won't be - // executed. - - execute_atexit_fcns (); - - // Do this explicitly so that destructors for mex file objects - // are called, so that functions registered with mexAtExit are - // called. - OCTAVE_SAFE_CALL (m_symbol_table.clear_mex_functions, ()); - - OCTAVE_SAFE_CALL (command_editor::restore_terminal_state, ()); - - OCTAVE_SAFE_CALL (m_history_system.write_timestamp, ()); - - if (! command_history::ignoring_entries ()) - OCTAVE_SAFE_CALL (command_history::clean_up_and_save, ()); - - OCTAVE_SAFE_CALL (m_gh_manager->close_all_figures, ()); - - m_gtk_manager.unload_all_toolkits (); - - // FIXME: May still need something like this to ensure that - // destructors for class objects will run properly. Should that be - // done earlier? Before or after atexit functions are executed? - m_symbol_table.cleanup (); - - OCTAVE_SAFE_CALL (sysdep_cleanup, ()); - - OCTAVE_SAFE_CALL (flush_stdout, ()); - - // Don't call singleton_cleanup_list::cleanup until we have the - // problems with registering/unregistering types worked out. For - // example, uncomment the following line, then use the make_int - // function from the examples directory to create an integer - // object and then exit Octave. Octave should crash with a - // segfault when cleaning up the typinfo singleton. We need some - // way to force new octave_value_X types that are created in - // .oct files to be unregistered when the .oct file shared library - // is unloaded. - // - // OCTAVE_SAFE_CALL (singleton_cleanup_list::cleanup, ()); - } - - void interpreter::execute_atexit_fcns (void) - { - // Prevent atexit functions from adding new functions to the list. - m_executing_atexit = true; - - while (! m_atexit_fcns.empty ()) - { - std::string fcn = m_atexit_fcns.front (); - - m_atexit_fcns.pop_front (); - - OCTAVE_SAFE_CALL (feval, (fcn, octave_value_list (), 0)); - - OCTAVE_SAFE_CALL (flush_stdout, ()); - } - } - tree_evaluator& interpreter::get_evaluator (void) { return m_evaluator;
--- a/libinterp/corefcn/interpreter.h Thu May 21 11:23:24 2020 -0400 +++ b/libinterp/corefcn/interpreter.h Fri May 22 12:54:13 2020 -0400 @@ -153,6 +153,8 @@ int execute (void); + void shutdown (void); + bool interactive (void) const { return m_interactive; @@ -491,8 +493,6 @@ int main_loop (void); - void cleanup (void); - void execute_atexit_fcns (void); application *m_app_context;