Re: [RFA] Make continuations per-thread.

[Pedro Alves <pedro@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 1378 bytes --]

A Friday 02 May 2008 04:00:12, Daniel Jacobowitz wrote:
> On Thu, Apr 24, 2008 at 07:45:38PM +0300, Vladimir Prus wrote:
> > Right now, continuations are global. This means that if we're doing
> > 'finish' in one thread, then we cannot do 'finish' or anything that
> > also uses continuations, in any other thread. This seems unnecessary
> > limitation, and this patch makes continuations per-thread.
> >
> > Further into non-stop series, it really allows me to do 'finish' or
> > 'step' in several threads independently.
> >
> > OK?
>
> Could you explain why this is safe?  We will do continuations for the
> thread which reports an event only.  So it seems like continuations
> for other threads will linger in their struct thread.
>
> For example, if we finish from one thread and hit a breakpoint in
> another thread before the finish returns.

That's true.  Attached is what we have next on the non-stop
series to fix that.  I'm not thrilled by it, but there are intermediate
context switches in handle_inferior_event that make it much uglier to try
to not make it centralized.  This is one of the things that gets much
better looking when we switch completelly to always-a-thread, and
get rid of context-switching.  I'm introducing another variable, instead of
using previous_inferior_ptid, which would be a good candidate, but I have
other plans for it.

-- 
Pedro Alves

[-- Attachment #2: switch_context_for_intermediate_continuations.patch --]
[-- Type: text/x-diff, Size: 3689 bytes --]

2008-05-02  Pedro Alves  <pedro@codesourcery.com>

	* inferior.h (step_ptid): Declare variable.
	* infcmd.c (step_1): Set step_ptid.
	* infrun.c (step_ptid): New variable.
	(fetch_inferior_event): Run the intermediate continuations in the
	right context, and clear step_ptid.

---
 gdb/infcmd.c   |    1 +
 gdb/inferior.h |    4 ++++
 gdb/infrun.c   |   36 +++++++++++++++++++++++++++++++++++-
 3 files changed, 40 insertions(+), 1 deletion(-)

Index: src/gdb/inferior.h
===================================================================
--- src.orig/gdb/inferior.h	2008-05-02 12:32:10.000000000 +0100
+++ src/gdb/inferior.h	2008-05-02 12:32:11.000000000 +0100
@@ -371,6 +371,10 @@ enum stop_kind
 
 extern enum stop_kind stop_soon;
 
+/* The thread we are executing a `step n'-like operation on.  Used in
+   all-stop mode to run the intermediate-continuations.  */
+extern ptid_t step_ptid;
+
 /* Nonzero if proceed is being used for a "finish" command or a similar
    situation when stop_registers should be saved.  */
 
Index: src/gdb/infcmd.c
===================================================================
--- src.orig/gdb/infcmd.c	2008-05-02 12:32:10.000000000 +0100
+++ src/gdb/infcmd.c	2008-05-02 12:32:11.000000000 +0100
@@ -789,6 +789,7 @@ which has no line number information.\n"
      and handle them one at the time, through step_once(). */
   else
     {
+      step_ptid = inferior_ptid;
       step_once (skip_subroutines, single_inst, count);
       /* We are running, and the contination is installed.  It will
 	 disable the longjmp breakpoint as appropriate.  */
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2008-05-02 12:32:10.000000000 +0100
+++ src/gdb/infrun.c	2008-05-02 12:32:11.000000000 +0100
@@ -74,6 +74,8 @@ static void set_schedlock_func (char *ar
 
 struct execution_control_state;
 
+static void context_switch (struct execution_control_state *ecs);
+
 static int currently_stepping (struct execution_control_state *ecs);
 
 static void xdb_handle_command (char *args, int from_tty);
@@ -447,6 +449,10 @@ follow_exec (int pid, char *execd_pathna
    until after the `wait' in `wait_for_inferior'.  */
 static int singlestep_breakpoints_inserted_p = 0;
 
+/* The thread we are executing a `step n'-like operation on.  Used in
+   all-stop mode to run the intermediate-continuations.  */
+ptid_t step_ptid;
+
 /* The thread we inserted single-step breakpoints for.  */
 static ptid_t singlestep_ptid;
 
@@ -1521,11 +1527,39 @@ fetch_inferior_event (void *client_data)
     {
       delete_step_resume_breakpoint (&step_resume_breakpoint);
 
+      /* In all-stop mode, if an event in another thread stops a
+	 requested `step n'-like operation, we must still run the
+	 intermediate continuations.  Since continuations are per
+	 thread, we must context switch back to the stepping thread to
+	 run them.  */
+      if (!ptid_equal (step_ptid, null_ptid)
+	  && !ptid_equal (step_ptid, inferior_ptid)
+	  && in_thread_list (step_ptid)
+	  && in_thread_list (inferior_ptid))
+	{
+	  volatile struct gdb_exception e;
+	  ptid_t current_ptid = inferior_ptid;
+
+	  async_ecs->ptid = step_ptid;
+	  context_switch (async_ecs);
+
+	  TRY_CATCH (e, RETURN_MASK_ERROR)
+	    {
+	      do_all_intermediate_continuations (0);
+	    }
+
+	  async_ecs->ptid = current_ptid;
+	  context_switch (async_ecs);
+	}
+
       normal_stop ();
       if (step_multi && stop_step)
 	inferior_event_handler (INF_EXEC_CONTINUE, NULL);
       else
-	inferior_event_handler (INF_EXEC_COMPLETE, NULL);
+	{
+	  inferior_event_handler (INF_EXEC_COMPLETE, NULL);
+	  step_ptid = null_ptid;
+	}
     }
 }