Mercurial > octave
changeset 31723:9ab31fe294c1 stable
don't emit signals for invalid objects in interpreter callbacks (bug #62863)
Some interpreter_event callbacks emit Qt signals. The callback is
queued by the Octave interpreter and may be executed at
some arbitrary time. It is possible that the object for which the
signal is intended to be executed may be deleted before the callback
executes. If so, then the callback should not emit the signal. In
some cases, execution of the entire callback function may be skipped.
In others, we still need to perform actions in the interpreter but
must skip the signals for Qt objects that no longer exist.
* command-widget.cc, file-editor-tab.cc, file-editor.cc,
octave-qscintilla.cc, main-window.cc, set-path-model.cc,
variable-editor-model.cc, variable-editor.cc: In all interpreter_event
callback functions that emit Qt signals, use a QPointer object to
protect the "this" pointer and check that it is still valid when the
callback executes.
| author | John W. Eaton <jwe@octave.org> |
|---|---|
| date | Wed, 11 Jan 2023 12:16:25 -0500 |
| parents | b4bde1e47dde |
| children | 340d016c2edf 3678187f4355 |
| files | libgui/src/command-widget.cc libgui/src/m-editor/file-editor-tab.cc libgui/src/m-editor/file-editor.cc libgui/src/m-editor/octave-qscintilla.cc libgui/src/main-window.cc libgui/src/set-path-model.cc libgui/src/variable-editor-model.cc libgui/src/variable-editor.cc |
| diffstat | 8 files changed, 186 insertions(+), 2 deletions(-) [+] |
line wrap: on
line diff
--- a/libgui/src/command-widget.cc Tue Jan 10 14:32:20 2023 -0800 +++ b/libgui/src/command-widget.cc Wed Jan 11 12:16:25 2023 -0500 @@ -92,11 +92,23 @@ void command_widget::init_command_prompt () { + // The interpreter_event callback function below emits a signal. + // Because we don't control when that happens, use a guarded pointer + // so that the callback can abort if this object is no longer valid. + + QPointer<command_widget> this_cw (this); + emit interpreter_event ([=] (interpreter& interp) { // INTERPRETER THREAD + // We can skip the entire callback function because it does not + // make any changes to the interpreter state. + + if (this_cw.isNull ()) + return; + event_manager& evmgr = interp.get_event_manager (); input_system& input_sys = interp.get_input_system (); std::string prompt = input_sys.PS1 (); @@ -123,14 +135,27 @@ void command_widget::process_input_line (const QString& input_line) { + // The interpreter_event callback function below emits a signal. + // Because we don't control when that happens, use a guarded pointer + // so that the callback can abort if this object is no longer valid. + + QPointer<command_widget> this_cw (this); + emit interpreter_event ([=] (interpreter& interp) { // INTERPRETER THREAD + // If THIS_CW is no longer valid, we still want to parse and + // execute INPUT_LINE but we can't emit the signals associated + // with THIS_CW. + interp.parse_and_execute (input_line.toStdString (), m_incomplete_parse); + if (this_cw.isNull ()) + return; + event_manager& evmgr = interp.get_event_manager (); input_system& input_sys = interp.get_input_system ();
--- a/libgui/src/m-editor/file-editor-tab.cc Tue Jan 10 14:32:20 2023 -0800 +++ b/libgui/src/m-editor/file-editor-tab.cc Wed Jan 11 12:16:25 2023 -0500 @@ -431,11 +431,25 @@ if (ok && ! new_cond.isEmpty ()) { + // The interpreter_event callback function below emits a signal. + // Because we don't control when that happens, use a guarded + // pointer so that the callback can abort if this object is no + // longer valid. + + QPointer<file_editor_tab> this_fetab (this); + emit interpreter_event ([=] (interpreter& interp) { // INTERPRETER THREAD + // We are intentionally skipping any side effects that may + // occur in the evaluation of NEW_COND if THIS_FETAB is no + // longer valid. + + if (this_fetab.isNull ()) + return; + error_system& es = interp.get_error_system (); unwind_protect frame; @@ -816,11 +830,25 @@ if (m_is_octave_file) { + // The interpreter_event callback function below emits a + // signal. Because we don't control when that happens, + // use a guarded pointer so that the callback can abort if + // this object is no longer valid. + + QPointer<file_editor_tab> this_fetab (this); + emit interpreter_event ([=] (interpreter& interp) { // INTERPRETER THREAD + // We can skip the entire callback function because it + // does not make any changes to the interpreter + // state. + + if (this_fetab.isNull ()) + return; + QStringList api_entries; octave_value_list tmp = Fiskeyword (); @@ -1359,11 +1387,21 @@ void file_editor_tab::add_breakpoint_event (int line, const QString& cond) { + // The interpreter_event callback function below emits a signal. + // Because we don't control when that happens, use a guarded pointer + // so that the callback can abort if this object is no longer valid. + + QPointer<file_editor_tab> this_fetab (this); + emit interpreter_event ([=] (interpreter& interp) { // INTERPRETER THREAD + // If THIS_FETAB is no longer valid, we still want to set the + // breakpoint in the interpreter but we can't emit the signal + // associated with THIS_FETAB. + // FIXME: note duplication with the code in // handle_context_menu_break_condition. @@ -1372,6 +1410,8 @@ int lineno = bptab.add_breakpoint_in_file (m_file_name.toStdString (), line, cond.toStdString ()); + if (this_fetab.isNull ()) + return; if (lineno) emit maybe_remove_next (lineno); @@ -2013,11 +2053,23 @@ // Create and queue the command object. + // The interpreter_event callback function below emits a signal. + // Because we don't control when that happens, use a guarded pointer + // so that the callback can abort if this object is no longer valid. + + QPointer<file_editor_tab> this_fetab (this); + emit interpreter_event ([=] (interpreter& interp) { // INTERPRETER THREAD + // We can skip the entire callback function because it does not + // make any changes to the interpreter state. + + if (this_fetab.isNull ()) + return; + octave_value_list argout = Fdbstatus (interp, ovl (), 1); connect (this, &file_editor_tab::update_breakpoints_signal, @@ -2076,11 +2128,22 @@ if (ans == QMessageBox::Save) { + // The interpreter_event callback function below emits a signal. + // Because we don't control when that happens, use a guarded + // pointer so that the callback can abort if this object is no + // longer valid. + + QPointer<file_editor_tab> this_fetab (this); + emit interpreter_event ([=] (interpreter& interp) { // INTERPRETER THREAD + // If THIS_FETAB is no longer valid, we still want to perform + // the actions in the interpreter but we can't emit the signal + // associated with THIS_FETAB. + tree_evaluator& tw = interp.get_evaluator (); tw.dbquit (true); @@ -2093,6 +2156,9 @@ symtab.clear_user_function (std_base_name); + if (this_fetab.isNull ()) + return; + emit do_save_file_signal (file_to_save, remove_on_success, restore_breakpoints); }); @@ -2131,11 +2197,27 @@ file_to_save = file_info.canonicalFilePath (); QString base_name = file_info.baseName (); + // The interpreter_event callback function below emits a signal. + // Because we don't control when that happens, use a guarded + // pointer so that the callback can abort if this object is no + // longer valid. + + QPointer<file_editor_tab> this_fetab (this); + emit interpreter_event ([=] (interpreter& interp) { // INTERPRETER THREAD + // We are intentionally skipping any side effects that may + // occur in the callback function if THIS_FETAB is no longer + // valid. If the editor tab has disappeared, there is not + // much point in reloading the function to restore breakpoint + // info in the GUI. + + if (this_fetab.isNull ()) + return; + // Force reloading of a file after it is saved. // This is needed to get the right line numbers for // breakpoints (bug #46632).
--- a/libgui/src/m-editor/file-editor.cc Tue Jan 10 14:32:20 2023 -0800 +++ b/libgui/src/m-editor/file-editor.cc Wed Jan 11 12:16:25 2023 -0500 @@ -681,11 +681,25 @@ void file_editor::request_run_file (bool) { + // The interpreter_event callback function below emits a signal. + // Because we don't control when that happens, use a guarded pointer + // so that the callback can abort if this object is no longer valid. + + QPointer<file_editor> this_fe (this); + emit interpreter_event ([=] (interpreter& interp) { // INTERPRETER THREAD + // If THIS_FE is no longer valid, skip the entire callback + // function. With the way things are implemented now, we can't + // run the contents of a file unless the file editor and the + // corresponding file editor tab are still valid. + + if (this_fe.isNull ()) + return; + // Act as though this action was entered at the command propmt // so that the interpreter will check for updated file time // stamps.
--- a/libgui/src/m-editor/octave-qscintilla.cc Tue Jan 10 14:32:20 2023 -0800 +++ b/libgui/src/m-editor/octave-qscintilla.cc Wed Jan 11 12:16:25 2023 -0500 @@ -899,6 +899,9 @@ { // INTERPRETER THREAD + if (tmp_hist.isNull ()) + return; + std::string opt = "-r"; std::string path = tmp_hist->fileName ().toStdString (); @@ -910,12 +913,24 @@ bool show_dbg_file = settings->value (ed_show_dbg_file).toBool (); settings->setValue (ed_show_dbg_file.key, false); +// The interpreter_event callback function below emits a signal. +// Because we don't control when that happens, use a guarded pointer +// so that the callback can abort if this object is no longer valid. + +QPointer<octave_qscintilla> this_oq (this); + // Let the interpreter execute the tmp file emit interpreter_event ([=] (interpreter& interp) { // INTERPRETER THREAD + // FIXME: For now, just skip the entire callback if THIS_OQ is no + // longer valid. Maybe there is a better way to do this job? + + if (this_oq.isNull ()) + return; + std::string file = tmp_file->fileName ().toStdString (); std::string pending_input = command_editor::get_current_line ();
--- a/libgui/src/main-window.cc Tue Jan 10 14:32:20 2023 -0800 +++ b/libgui/src/main-window.cc Wed Jan 11 12:16:25 2023 -0500 @@ -1414,11 +1414,23 @@ const QString& curr_dir, int line) { + // The interpreter_event callback function below emits a signal. + // Because we don't control when that happens, use a guarded pointer + // so that the callback can abort if this object is no longer valid. + + QPointer<main_window> this_mw (this); + emit interpreter_event ([=] (interpreter& interp) { // INTERPRETER THREAD + // We can skip the entire callback function because it does not + // make any changes to the interpreter state. + + if (this_mw.isNull ()) + return; + // Split possible subfunctions QStringList fcn_list = fname.split ('>'); QString fcn_name = fcn_list.at (0) + ".m"; @@ -1481,7 +1493,6 @@ filename = file.canonicalFilePath (); // private function exists else message = tr ("Can not find function %1"); // no file found - } }
--- a/libgui/src/set-path-model.cc Tue Jan 10 14:32:20 2023 -0800 +++ b/libgui/src/set-path-model.cc Wed Jan 11 12:16:25 2023 -0500 @@ -32,6 +32,7 @@ #include <QFileIconProvider> #include <QMessageBox> +#include <QPointer> #include <QtAlgorithms> #include "qt-interpreter-events.h" @@ -282,11 +283,23 @@ void set_path_model::path_to_model (void) { + // The interpreter_event callback function below emits a signal. + // Because we don't control when that happens, use a guarded pointer + // so that the callback can abort if this object is no longer valid. + + QPointer<set_path_model> this_spm (this); + emit interpreter_event ([=] (interpreter& interp) { // INTERPRETER THREAD + // We can skip the entire callback function because it does not + // make any changes to the interpreter state. + + if (this_spm.isNull ()) + return; + load_path& lp = interp.get_load_path (); std::list<std::string> dir_list = lp.dir_list ();
--- a/libgui/src/variable-editor-model.cc Tue Jan 10 14:32:20 2023 -0800 +++ b/libgui/src/variable-editor-model.cc Wed Jan 11 12:16:25 2023 -0500 @@ -33,6 +33,7 @@ #include <QLabel> #include <QMap> #include <QMessageBox> +#include <QPointer> #include <QString> #include <QTableView> @@ -1023,11 +1024,23 @@ std::string expr = os.str (); + // The interpreter_event callback function below emits a signal. + // Because we don't control when that happens, use a guarded pointer + // so that the callback can abort if this object is no longer valid. + + QPointer<variable_editor_model> this_vem (this); + emit interpreter_event ([=] (interpreter& interp) { // INTERPRETER THREAD + // We are intentionally skipping any side effects that may occur + // in the evaluation of EXPR if THIS_VEM is no longer valid. + + if (this_vem.isNull ()) + return; + try { int parse_status = 0;
--- a/libgui/src/variable-editor.cc Tue Jan 10 14:32:20 2023 -0800 +++ b/libgui/src/variable-editor.cc Wed Jan 11 12:16:25 2023 -0500 @@ -458,12 +458,24 @@ return; } + // The interpreter_event callback function below emits a signal. + // Because we don't control when that happens, use a guarded pointer + // so that the callback can abort if this object is no longer valid. + + QPointer<variable_editor_stack> this_ves (this); + // No format given, test save default options emit interpreter_event ([=] (interpreter& interp) { // INTERPRETER THREAD + // We can skip the entire callback function because it does not + // make any changes to the interpreter state. + + if (this_ves.isNull ()) + return; + octave_value_list argout = Fsave_default_options (interp, octave_value_list (), 1); QString save_opts = QString::fromStdString (argout(0).string_value ()); @@ -472,7 +484,6 @@ this, &variable_editor_stack::do_save); emit (do_save_signal (format_string, save_opts)); - }); }