changeset 25275:4adeabc1bbfe stable

improve Ctrl-C interrupt handling in the GUI (bug #53635) * main-window.h, main-window.cc (octave_interpreter::interrupt): Delete. (octave_interpreter::m_thread_manager): Delete member variable and all uses. * thread-manager.h, thread-manager.cc: Delete. * libgui/src/module.mk: Update. * main-window.h, main-window.cc (main_window::interrupt_interpreter): Delete. (main_window::construct): Don't connect command window interrupt signal to main window interrupt_interpreter slot. * octave-gui.cc (gui_application::execute): Call octave_block_interrupt_signal directly. * terminal-dock-widget.cc (terminal_dock_widget::terminal_interrupt): Set octave_signal_caught and octave_interrupt_state instead of emitting interrupt_signal.
author John W. Eaton <jwe@octave.org>
date Tue, 17 Apr 2018 21:40:31 -0400
parents 1f1e1e72e958
children 9eb3755b419b 692fbde19871
files libgui/src/main-window.cc libgui/src/main-window.h libgui/src/module.mk libgui/src/octave-gui.cc libgui/src/terminal-dock-widget.cc libgui/src/thread-manager.cc libgui/src/thread-manager.h
diffstat 7 files changed, 10 insertions(+), 228 deletions(-) [+]
line wrap: on
line diff
--- a/libgui/src/main-window.cc	Thu Apr 12 14:34:10 2018 -0400
+++ b/libgui/src/main-window.cc	Tue Apr 17 21:40:31 2018 -0400
@@ -88,13 +88,11 @@
 namespace octave
 {
   octave_interpreter::octave_interpreter (application *app_context)
-    : QObject (), m_thread_manager (), m_app_context (app_context)
+    : QObject (), m_app_context (app_context)
   { }
 
   void octave_interpreter::execute (void)
   {
-    m_thread_manager.register_current_thread ();
-
     // The application context owns the interpreter.
 
     interpreter& interp = m_app_context->create_interpreter ();
@@ -143,11 +141,6 @@
     emit octave_finished_signal (exit_status);
   }
 
-  void octave_interpreter::interrupt (void)
-  {
-    m_thread_manager.interrupt ();
-  }
-
   main_window::main_window (QWidget *p, gui_application *app_context)
     : QMainWindow (p), m_app_context (app_context),
       m_interpreter (new octave_interpreter (app_context)),
@@ -1653,11 +1646,6 @@
     m_clipboard->clear (QClipboard::Clipboard);
   }
 
-  void main_window::interrupt_interpreter (void)
-  {
-    m_interpreter->interrupt ();
-  }
-
   void main_window::disable_menu_shortcuts (bool disable)
   {
     QHash<QMenu*, QStringList>::const_iterator i = m_hash_menu_text.constBegin ();
@@ -1774,9 +1762,6 @@
         connect (m_variable_editor_window, SIGNAL (updated (void)),
                  this, SLOT (handle_variable_editor_update (void)));
 
-        connect (m_command_window, SIGNAL (interrupt_signal (void)),
-                 this, SLOT (interrupt_interpreter (void)));
-
         construct_menu_bar ();
 
         construct_tool_bar ();
--- a/libgui/src/main-window.h	Thu Apr 12 14:34:10 2018 -0400
+++ b/libgui/src/main-window.h	Tue Apr 17 21:40:31 2018 -0400
@@ -56,7 +56,6 @@
 #include "resource-manager.h"
 #include "terminal-dock-widget.h"
 #include "variable-editor.h"
-#include "thread-manager.h"
 #include "workspace-model.h"
 #include "workspace-view.h"
 
@@ -76,8 +75,6 @@
 
     ~octave_interpreter (void) = default;
 
-    void interrupt (void);
-
   signals:
 
     void octave_ready_signal (void);
@@ -91,8 +88,6 @@
 
   private:
 
-    thread_manager m_thread_manager;
-
     application *m_app_context;
   };
 
@@ -269,8 +264,6 @@
     void clear_clipboard ();
     //!@}
 
-    void interrupt_interpreter (void);
-
     //! Returns a list of dock widgets.
 
     QList<octave_dock_widget *> get_dock_widget_list (void)
--- a/libgui/src/module.mk	Thu Apr 12 14:34:10 2018 -0400
+++ b/libgui/src/module.mk	Tue Apr 17 21:40:31 2018 -0400
@@ -176,7 +176,6 @@
   %reldir%/settings-dialog.h \
   %reldir%/shortcut-manager.h \
   %reldir%/tab-bar.h \
-  %reldir%/thread-manager.h \
   %reldir%/terminal-dock-widget.h \
   %reldir%/color-picker.h \
   %reldir%/welcome-wizard.h \
@@ -211,7 +210,6 @@
   %reldir%/settings-dialog.cc \
   %reldir%/shortcut-manager.cc \
   %reldir%/tab-bar.cc \
-  %reldir%/thread-manager.cc \
   %reldir%/terminal-dock-widget.cc \
   %reldir%/color-picker.cc \
   %reldir%/welcome-wizard.cc \
--- a/libgui/src/octave-gui.cc	Thu Apr 12 14:34:10 2018 -0400
+++ b/libgui/src/octave-gui.cc	Tue Apr 17 21:40:31 2018 -0400
@@ -42,6 +42,7 @@
 #include "lo-utils.h"
 #include "oct-env.h"
 #include "oct-syscalls.h"
+#include "signal-wrappers.h"
 
 #include "builtin-defun-decls.h"
 #include "defaults.h"
@@ -54,7 +55,6 @@
 #include "octave-gui.h"
 #include "resource-manager.h"
 #include "shortcut-manager.h"
-#include "thread-manager.h"
 #include "welcome-wizard.h"
 
 // Disable all Qt messages by default.
@@ -84,7 +84,7 @@
 
   int gui_application::execute (void)
   {
-    thread_manager::block_interrupt_signal ();
+    octave_block_interrupt_signal ();
 
     set_application_id ();
 
--- a/libgui/src/terminal-dock-widget.cc	Thu Apr 12 14:34:10 2018 -0400
+++ b/libgui/src/terminal-dock-widget.cc	Tue Apr 17 21:40:31 2018 -0400
@@ -30,6 +30,9 @@
 #include "terminal-dock-widget.h"
 #include "resource-manager.h"
 
+#include "sighandlers.h"
+#include "quit.h"
+
 namespace octave
 {
   terminal_dock_widget::terminal_dock_widget (QWidget *p)
@@ -103,6 +106,9 @@
 
   void terminal_dock_widget::terminal_interrupt (void)
   {
-    emit interrupt_signal ();
+    // FIXME: Protect with mutex?
+
+    octave_signal_caught = 1;
+    octave_interrupt_state++;
   }
 }
--- a/libgui/src/thread-manager.cc	Thu Apr 12 14:34:10 2018 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,123 +0,0 @@
-/*
-
-Copyright (C) 2013-2018 John W. Eaton
-
-This file is part of Octave.
-
-Octave is free software: you can redistribute it and/or modify it
-under the terms of the GNU General Public License as published by
-the Free Software Foundation, either version 3 of the License, or
-(at your option) any later version.
-
-Octave is distributed in the hope that it will be useful, but
-WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-GNU General Public License for more details.
-
-You should have received a copy of the GNU General Public License
-along with Octave; see the file COPYING.  If not, see
-<https://www.gnu.org/licenses/>.
-
-*/
-
-#if defined (HAVE_CONFIG_H)
-#  include "config.h"
-#endif
-
-#if defined (OCTAVE_USE_WINDOWS_API)
-#  include <windows.h>
-#else
-#  include <pthread.h>
-#endif
-
-#include "signal-wrappers.h"
-
-#include "thread-manager.h"
-
-namespace octave
-{
-#if defined (OCTAVE_USE_WINDOWS_API)
-
-  class windows_thread_manager : public base_thread_manager
-  {
-  public:
-
-    windows_thread_manager (void) : base_thread_manager () { }
-
-    void register_current_thread (void) { }
-
-    void interrupt (void)
-    {
-      GenerateConsoleCtrlEvent (CTRL_C_EVENT, 0);
-    }
-  };
-
-#else
-
-  class pthread_thread_manager : public base_thread_manager
-  {
-  public:
-
-    pthread_thread_manager (void)
-      : base_thread_manager (), m_my_thread (), m_initialized (false)
-    { }
-
-    void register_current_thread (void)
-    {
-      m_my_thread = pthread_self ();
-      m_initialized = true;
-    }
-
-    void interrupt (void)
-    {
-      if (m_initialized)
-        {
-          // Send SIGINT to all other processes in our process group.
-          // This is needed to interrupt calls to system (), for example.
-
-          // FIXME: What happens if some code inside a
-          // {BEGIN,END}_INTERRUPT_IMMEDIATELY_IN_FOREIGN_CODE block starts
-          // additional threads and one of those happens to catch this signal?
-          // Would the interrupt handler and the subsequent longjmp and exception
-          // all be executed in the wrong thread?  If so, is there any way to
-          // prevent that from happening?
-
-          int sigint;
-          octave_get_sig_number ("SIGINT", &sigint);
-
-          octave_kill_wrapper (0, sigint);
-        }
-    }
-
-  private:
-
-    pthread_t m_my_thread;
-
-    bool m_initialized;
-  };
-
-#endif
-
-  thread_manager::thread_manager (void)
-    : m_rep (thread_manager::create_rep ())
-  { }
-
-  void thread_manager::block_interrupt_signal (void)
-  {
-    octave_block_interrupt_signal ();
-  }
-
-  void thread_manager::unblock_interrupt_signal (void)
-  {
-    octave_unblock_interrupt_signal ();
-  }
-
-  base_thread_manager * thread_manager::create_rep (void)
-  {
-#if defined (OCTAVE_USE_WINDOWS_API)
-    return new windows_thread_manager ();
-#else
-    return new pthread_thread_manager ();
-#endif
-  }
-}
--- a/libgui/src/thread-manager.h	Thu Apr 12 14:34:10 2018 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,77 +0,0 @@
-/*
-
-Copyright (C) 2013-2018 John W. Eaton
-
-This file is part of Octave.
-
-Octave is free software: you can redistribute it and/or modify it
-under the terms of the GNU General Public License as published by
-the Free Software Foundation, either version 3 of the License, or
-(at your option) any later version.
-
-Octave is distributed in the hope that it will be useful, but
-WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-GNU General Public License for more details.
-
-You should have received a copy of the GNU General Public License
-along with Octave; see the file COPYING.  If not, see
-<https://www.gnu.org/licenses/>.
-
-*/
-
-#if ! defined (octave_thread_manager_h)
-#define octave_thread_manager_h 1
-
-#include "octave-config.h"
-
-#include <memory>
-
-namespace octave
-{
-  class base_thread_manager
-  {
-  public:
-
-    friend class thread_manager;
-
-    base_thread_manager (void) = default;
-
-    base_thread_manager (const base_thread_manager&) = default;
-
-    virtual ~base_thread_manager (void) = default;
-
-    virtual void register_current_thread (void) = 0;
-
-    virtual void interrupt (void) = 0;
-  };
-
-  class thread_manager
-  {
-  public:
-
-    thread_manager (void);
-
-    ~thread_manager (void) = default;
-
-    thread_manager (const thread_manager&) = default;
-
-    thread_manager& operator = (const thread_manager&) = default;
-
-    void register_current_thread (void) { m_rep->register_current_thread (); }
-
-    void interrupt (void) { m_rep->interrupt (); }
-
-    static void block_interrupt_signal (void);
-
-    static void unblock_interrupt_signal (void);
-
-  private:
-
-    std::shared_ptr<base_thread_manager> m_rep;
-
-    static base_thread_manager * create_rep (void);
-  };
-}
-
-#endif