changeset 29282:0280fd549502

make debugging possible in server mode * event-manager.h, event-manager.cc (event_manager::gui_event_queue): Use a stack of event_queue objects. * pt-eval.h, pt-eval.cc (debugger::server_loop): New function. (debugger::repl): Handle case of execution in server mode. (tree_evaluator::set_parser): Allow setting m_parser. (tree_evaluator::server_mode): Allow setting server_mode.
author John W. Eaton <jwe@octave.org>
date Thu, 07 Jan 2021 01:30:18 -0500
parents 6dd456257d81
children aa645ebc7b29
files libinterp/corefcn/event-manager.cc libinterp/corefcn/event-manager.h libinterp/parse-tree/pt-eval.cc libinterp/parse-tree/pt-eval.h
diffstat 4 files changed, 330 insertions(+), 147 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/event-manager.cc	Thu Jan 07 00:01:18 2021 -0500
+++ b/libinterp/corefcn/event-manager.cc	Thu Jan 07 01:30:18 2021 -0500
@@ -55,6 +55,7 @@
       event_queue_mutex (new mutex ()), gui_event_queue (),
       debugging (false), link_enabled (false)
   {
+    push_event_queue ();
     command_editor::add_event_hook (readline_event_hook);
   }
 
@@ -98,10 +99,10 @@
           disable ();
 
         event_queue_mutex->lock ();
+        std::shared_ptr<event_queue> evq = gui_event_queue.top ();
+        event_queue_mutex->unlock ();
 
-        gui_event_queue.run ();
-
-        event_queue_mutex->unlock ();
+        evq->run ();
       }
   }
 
@@ -110,10 +111,49 @@
     if (enabled ())
       {
         event_queue_mutex->lock ();
+        std::shared_ptr<event_queue> evq = gui_event_queue.top ();
+        event_queue_mutex->unlock ();
 
-        gui_event_queue.discard ();
+        evq->discard ();
+      }
+  }
+
+  void event_manager::push_event_queue (void)
+  {
+    std::shared_ptr<event_queue> evq (new event_queue ());
+    gui_event_queue.push (evq);
+  }
 
-        event_queue_mutex->unlock ();
+  void event_manager::pop_event_queue (void)
+  {
+    // FIXME: Should we worry about the possibility of events remaining
+    // in the queue when we pop back to the previous queue?  If so, then
+    // we will probably want to push them on to the front of the
+    // previous queue so they will be executed before any other events
+    // that were in the previous queue.  This case could happen if
+    // graphics callback functions were added to the event queue during a
+    // debug session just after a dbcont command was added but before it
+    // executed and brought us here, for example.
+
+    std::shared_ptr<event_queue> evq = gui_event_queue.top ();
+    gui_event_queue.pop ();
+  }
+
+  void event_manager::post_event (const fcn_callback& fcn)
+  {
+    if (enabled ())
+      {
+        std::shared_ptr<event_queue> evq = gui_event_queue.top ();
+        evq->add (fcn);
+      }
+  }
+
+  void event_manager::post_event (const meth_callback& meth)
+  {
+    if (enabled ())
+      {
+        std::shared_ptr<event_queue> evq = gui_event_queue.top ();
+        evq->add (std::bind (meth, std::ref (m_interpreter)));
       }
   }
 
--- a/libinterp/corefcn/event-manager.h	Thu Jan 07 00:01:18 2021 -0500
+++ b/libinterp/corefcn/event-manager.h	Thu Jan 07 01:30:18 2021 -0500
@@ -31,6 +31,7 @@
 #include <functional>
 #include <list>
 #include <memory>
+#include <stack>
 #include <string>
 
 #include "oct-mutex.h"
@@ -302,17 +303,11 @@
     // The queued functions are executed when the interpreter is
     // otherwise idle.
 
-    void post_event (const fcn_callback& fcn)
-    {
-      if (enabled ())
-        gui_event_queue.add (fcn);
-    }
+    void push_event_queue (void);
+    void pop_event_queue (void);
 
-    void post_event (const meth_callback& meth)
-    {
-      if (enabled ())
-        gui_event_queue.add (std::bind (meth, std::ref (m_interpreter)));
-    }
+    OCTINTERP_API void post_event (const fcn_callback& fcn);
+    OCTINTERP_API void post_event (const meth_callback& meth);
 
     // The following functions correspond to the virtual fuunctions in
     // the interpreter_events class.  They provide a way for the
@@ -652,8 +647,17 @@
     // Semaphore to lock access to the event queue.
     mutex *event_queue_mutex;
 
-    // Event Queue.
-    event_queue gui_event_queue;
+    // Event Queue.  We use a stack so that we can handle evaluation in
+    // the debugger when we are executing in server mode.  In server
+    // mode, code is evaluated from inside the event queue.  So when the
+    // evaluator reaches a breakpoint, the queue is already locked and
+    // executing an event function.  We can't just add a new command to the
+    // existing queue, so we need another one that can process new
+    // events generated at the debug prompt.  When execution continues
+    // (dbcont or dbstep, for example) we pop the queue and return to
+    // the previous point of execution.
+
+    std::stack<std::shared_ptr <event_queue>> gui_event_queue;
 
     bool debugging;
     bool link_enabled;
--- a/libinterp/parse-tree/pt-eval.cc	Thu Jan 07 00:01:18 2021 -0500
+++ b/libinterp/parse-tree/pt-eval.cc	Thu Jan 07 01:30:18 2021 -0500
@@ -111,6 +111,8 @@
         m_execution_mode (EX_NORMAL), m_in_debug_repl (false)
     { }
 
+    int server_loop (void);
+
     void repl (const std::string& prompt = "debug> ");
 
     bool in_debug_repl (void) const { return m_in_debug_repl; }
@@ -140,6 +142,115 @@
     bool m_in_debug_repl;
   };
 
+  // FIXME: Could the debugger server_loop and repl functions be merged
+  // with the corresponding tree_evaluator functions or do they need to
+  // remain separate?  They perform nearly the same functions.
+
+  int debugger::server_loop (void)
+  {
+    // Process events from the event queue.
+
+    tree_evaluator& tw = m_interpreter.get_evaluator ();
+
+    void (tree_evaluator::*server_mode_fptr) (bool)
+      = &tree_evaluator::server_mode;
+    unwind_action act (server_mode_fptr, &tw, true);
+
+    int exit_status = 0;
+
+    do
+      {
+        if (m_execution_mode == EX_CONTINUE || tw.dbstep_flag ())
+          break;
+
+        if (quitting_debugger ())
+          break;
+
+        try
+          {
+            // FIXME: Running the event queue should be decoupled from
+            // the command_editor.
+
+            // FIXME: Is it OK to use command_editor::run_event_hooks
+            // here?  It may run more than one queued function per call,
+            // and it seems that the checks at the top of the loop
+            // probably need to be done after each individual event
+            // function is executed.  For now, maybe the simplest thing
+            // would be to pass a predicate function (lambda expression)
+            // to the command_editor::run_event_hooks and have it check
+            // that and break out of the eval loop(s) if the condition
+            // is met?
+
+            // FIXME: We should also use a condition variable to manage
+            // the execution of entries in the queue and eliminate the
+            // need for the busy-wait loop.
+
+            command_editor::run_event_hooks ();
+
+            octave::sleep (0.1);
+          }
+        catch (const interrupt_exception&)
+          {
+            m_interpreter.recover_from_exception ();
+
+            // Required newline when the user does Ctrl+C at the prompt.
+            if (m_interpreter.interactive ())
+              octave_stdout << "\n";
+          }
+        catch (const index_exception& e)
+          {
+            m_interpreter.recover_from_exception ();
+
+            std::cerr << "error: unhandled index exception: "
+                      << e.message () << " -- trying to return to prompt"
+                      << std::endl;
+          }
+        catch (const execution_exception& ee)
+          {
+            error_system& es = m_interpreter.get_error_system ();
+
+            es.save_exception (ee);
+            es.display_exception (ee, std::cerr);
+
+            if (m_interpreter.interactive ())
+              {
+                m_interpreter.recover_from_exception ();
+              }
+            else
+              {
+                // We should exit with a nonzero status.
+                exit_status = 1;
+                break;
+              }
+          }
+        catch (const quit_debug_exception& qde)
+          {
+            if (qde.all ())
+              throw;
+
+            // Continue in this debug level.
+          }
+        catch (const std::bad_alloc&)
+          {
+            m_interpreter.recover_from_exception ();
+
+            std::cerr << "error: out of memory -- trying to return to prompt"
+                      << std::endl;
+          }
+      }
+    while (exit_status == 0);
+
+    if (exit_status == EOF)
+      {
+        if (m_interpreter.interactive ())
+          octave_stdout << "\n";
+
+        exit_status = 0;
+      }
+
+    return exit_status;
+  }
+
   void debugger::repl (const std::string& prompt_arg)
   {
     unwind_protect frame;
@@ -173,6 +284,8 @@
 
     input_system& input_sys = m_interpreter.get_input_system ();
 
+    event_manager& evmgr = m_interpreter.get_event_manager ();
+
     if (! fcn_nm.empty ())
       {
         if (input_sys.gud_mode ())
@@ -194,8 +307,6 @@
                 frm->display_stopped_in_message (buf);
               }
 
-            event_manager& evmgr = m_interpreter.get_event_manager ();
-
             evmgr.enter_debugger_event (fcn_nm, fcn_file_nm, curr_debug_line);
 
             evmgr.set_workspace ();
@@ -221,146 +332,167 @@
 
     std::string stopped_in_msg = buf.str ();
 
-    if (! stopped_in_msg.empty ())
-      std::cerr << stopped_in_msg << std::endl;
-
-    std::string tmp_prompt = prompt_arg;
-    if (m_level > 0)
-      tmp_prompt = "[" + std::to_string (m_level) + "]" + prompt_arg;
-
-    frame.add (&input_system::set_PS1, &input_sys, input_sys.PS1 ());
-    input_sys.PS1 (tmp_prompt);
-
-    if (! m_interpreter.interactive ())
+    if (m_interpreter.server_mode ())
       {
-        void (interpreter::*interactive_fptr) (bool)
-          = &interpreter::interactive;
-        frame.add (interactive_fptr, &m_interpreter,
-                   m_interpreter.interactive ());
-
-        m_interpreter.interactive (true);
-
-        // FIXME: should debugging be possible in an embedded
-        // interpreter?
-
-        application *app = application::app ();
-
-        if (app)
+        if (! stopped_in_msg.empty ())
+          octave_stdout << stopped_in_msg << std::endl;
+
+        evmgr.push_event_queue ();
+
+        frame.add (&event_manager::pop_event_queue, &evmgr);
+
+        frame.add (&tree_evaluator::set_parser, &tw, tw.get_parser ());
+
+        std::shared_ptr<push_parser>
+          debug_parser (new push_parser (m_interpreter));
+
+        tw.set_parser (debug_parser);
+
+        server_loop ();
+      }
+    else
+      {
+        if (! stopped_in_msg.empty ())
+          std::cerr << stopped_in_msg << std::endl;
+
+        std::string tmp_prompt = prompt_arg;
+        if (m_level > 0)
+          tmp_prompt = "[" + std::to_string (m_level) + "]" + prompt_arg;
+
+        frame.add (&input_system::set_PS1, &input_sys, input_sys.PS1 ());
+        input_sys.PS1 (tmp_prompt);
+
+        if (! m_interpreter.interactive ())
           {
-            void (application::*forced_interactive_fptr) (bool)
-              = &application::forced_interactive;
-            frame.add (forced_interactive_fptr, app,
-                       app->forced_interactive ());
-
-            app->forced_interactive (true);
+            void (interpreter::*interactive_fptr) (bool)
+              = &interpreter::interactive;
+            frame.add (interactive_fptr, &m_interpreter,
+                       m_interpreter.interactive ());
+
+            m_interpreter.interactive (true);
+
+            // FIXME: should debugging be possible in an embedded
+            // interpreter?
+
+            application *app = application::app ();
+
+            if (app)
+              {
+                void (application::*forced_interactive_fptr) (bool)
+                  = &application::forced_interactive;
+                frame.add (forced_interactive_fptr, app,
+                           app->forced_interactive ());
+
+                app->forced_interactive (true);
+              }
           }
-      }
-
-#if defined (OCTAVE_ENABLE_COMMAND_LINE_PUSH_PARSER)
-
-    input_reader reader (m_interpreter);
-
-    push_parser debug_parser (m_interpreter);
-
-#else
-
-    parser debug_parser (m_interpreter);
-
-#endif
-
-    error_system& es = m_interpreter.get_error_system ();
-
-    while (m_in_debug_repl)
-      {
-        if (m_execution_mode == EX_CONTINUE || tw.dbstep_flag ())
-          break;
-
-        if (quitting_debugger ())
-          break;
-
-        try
-          {
-            debug_parser.reset ();
 
 #if defined (OCTAVE_ENABLE_COMMAND_LINE_PUSH_PARSER)
 
-            int retval = 0;
-
-            std::string prompt
-              = command_editor::decode_prompt_string (tmp_prompt);
-
-            do
-              {
-                bool eof = false;
-                std::string input_line = reader.get_input (prompt, eof);
-
-                if (eof)
-                  {
-                    retval = EOF;
-                    break;
-                  }
-
-                retval = debug_parser.run (input_line, false);
-
-                prompt = command_editor::decode_prompt_string (input_sys.PS2 ());
-              }
-            while (retval < 0);
+        input_reader reader (m_interpreter);
+
+        push_parser debug_parser (m_interpreter);
 
 #else
 
-            int retval = debug_parser.run ();
+        parser debug_parser (m_interpreter);
 
 #endif
-            if (command_editor::interrupt (false))
+
+        error_system& es = m_interpreter.get_error_system ();
+
+        while (m_in_debug_repl)
+          {
+            if (m_execution_mode == EX_CONTINUE || tw.dbstep_flag ())
+              break;
+
+            if (quitting_debugger ())
+              break;
+
+            try
               {
-                // Break regardless of m_execution_mode value.
-
-                quitting_debugger ();
-
-                break;
-              }
-            else
-              {
-                if (retval == 0)
+                debug_parser.reset ();
+
+#if defined (OCTAVE_ENABLE_COMMAND_LINE_PUSH_PARSER)
+
+                int retval = 0;
+
+                std::string prompt
+                  = command_editor::decode_prompt_string (tmp_prompt);
+
+                do
+                  {
+                    bool eof = false;
+                    std::string input_line = reader.get_input (prompt, eof);
+
+                    if (eof)
+                      {
+                        retval = EOF;
+                        break;
+                      }
+
+                    retval = debug_parser.run (input_line, false);
+
+                    prompt = command_editor::decode_prompt_string (input_sys.PS2 ());
+                  }
+                while (retval < 0);
+
+#else
+
+                int retval = debug_parser.run ();
+
+#endif
+                if (command_editor::interrupt (false))
                   {
-                    std::shared_ptr<tree_statement_list> stmt_list
-                      = debug_parser.statement_list ();
-
-                    if (stmt_list)
-                      stmt_list->accept (tw);
-
-                    if (octave_completion_matches_called)
-                      octave_completion_matches_called = false;
-
-                    // FIXME: the following statement is here because
-                    // the last command may have been a dbup, dbdown, or
-                    // dbstep command that changed the current debug
-                    // frame.  If so, we need to reset the current frame
-                    // for the call stack.  But is this right way to do
-                    // this job?  What if the statement list was
-                    // something like "dbup; dbstack"?  Will the call to
-                    // dbstack use the right frame?  If not, how can we
-                    // fix this problem?
-                    tw.goto_frame (tw.debug_frame ());
+                    // Break regardless of m_execution_mode value.
+
+                    quitting_debugger ();
+
+                    break;
                   }
-
-                octave_quit ();
+                else
+                  {
+                    if (retval == 0)
+                      {
+                        std::shared_ptr<tree_statement_list> stmt_list
+                          = debug_parser.statement_list ();
+
+                        if (stmt_list)
+                          stmt_list->accept (tw);
+
+                        if (octave_completion_matches_called)
+                          octave_completion_matches_called = false;
+
+                        // FIXME: the following statement is here because
+                        // the last command may have been a dbup, dbdown, or
+                        // dbstep command that changed the current debug
+                        // frame.  If so, we need to reset the current frame
+                        // for the call stack.  But is this right way to do
+                        // this job?  What if the statement list was
+                        // something like "dbup; dbstack"?  Will the call to
+                        // dbstack use the right frame?  If not, how can we
+                        // fix this problem?
+                        tw.goto_frame (tw.debug_frame ());
+                      }
+
+                    octave_quit ();
+                  }
               }
-          }
-        catch (const execution_exception& ee)
-          {
-            es.save_exception (ee);
-            es.display_exception (ee, std::cerr);
-
-            // Ignore errors when in debugging mode;
-            m_interpreter.recover_from_exception ();
-          }
-        catch (const quit_debug_exception& qde)
-          {
-            if (qde.all ())
-              throw;
-
-            // Continue in this debug level.
+            catch (const execution_exception& ee)
+              {
+                es.save_exception (ee);
+                es.display_exception (ee, std::cerr);
+
+                // Ignore errors when in debugging mode;
+                m_interpreter.recover_from_exception ();
+              }
+            catch (const quit_debug_exception& qde)
+              {
+                if (qde.all ())
+                  throw;
+
+                // Continue in this debug level.
+              }
           }
       }
   }
@@ -621,12 +753,12 @@
   {
     // Process events from the event queue.
 
-    unwind_protect_var<bool> upv (m_server_mode, true);
+    unwind_protect_var<bool> upv1 (m_server_mode, true);
 
     m_exit_status = 0;
 
-    if (! m_parser)
-      m_parser = std::shared_ptr<push_parser> (new push_parser (m_interpreter));
+    std::shared_ptr<push_parser> parser (new push_parser (m_interpreter));
+    unwind_protect_var<std::shared_ptr<push_parser>> upv2 (m_parser, parser);
 
     do
       {
--- a/libinterp/parse-tree/pt-eval.h	Thu Jan 07 00:01:18 2021 -0500
+++ b/libinterp/parse-tree/pt-eval.h	Thu Jan 07 01:30:18 2021 -0500
@@ -157,6 +157,11 @@
       return m_parser;
     }
 
+    void set_parser (const std::shared_ptr<push_parser>& parser)
+    {
+      m_parser = parser;
+    }
+
     bool at_top_level (void) const;
 
     std::string mfilename (const std::string& opt = "") const;
@@ -175,6 +180,8 @@
 
     bool server_mode (void) const { return m_server_mode; }
 
+    void server_mode (bool arg) { m_server_mode = arg; }
+
     void eval (std::shared_ptr<tree_statement_list>& stmt_list,
                bool interactive);