[PUSHED] normal_stop: Extend and clarify comment. (was: Re: No thread context switch in non-stop mode???)

[Pedro Alves <palves at redhat dot com>]

On 03/17/2014 11:54 PM, Doug Evans wrote:
> Pedro Alves writes:


>  > > The above comment is a bit misleading since that code doesn't actually
>  > > change anything, it just prints a message notifying the user that the
>  > > current thread has changed.
>  > 
>  > Hmm, WDYM by prints a message?
> 
> I mean it prints a message, this message:
> 
>       printf_filtered (_("[Switching to %s]\n"),
> 		       target_pid_to_str (inferior_ptid));
> 
> This comment:
> 
>   /* In non-stop mode, we don't want GDB to switch threads behind the
>      user's back, to avoid races where the user is typing a command to
>      apply to thread x, but GDB switches to thread y before the user
>      finishes entering the command.  */
> 
> is misleading (to me) because while the wording is correct the
> place where it lives has nothing to do with implementing that.

> As I'm reading the comment I'm expecting the following code to make sure GDB
> doesn't switch threads behind the user's back, and since the following
> code is not doing that, and is just printing the "Switching to" message
> (given certain conditions (!non_stop && ...)),
> I'm left wondering if there's a bug.
> Could be missing something of course.

Ah, I see what you mean now.

> 
>  > If it weren't for the cleanup,
> 
> For clarity's sake, which cleanup?

I've applied the patch below.  Let's extend it further if still not
clear.

BTW, I think we should actually change the predicate that controls
whether GDB switches threads from non_stop, to "whether the user
has the prompt".  IOW, fg vs bg command.  Or YOW, sync_execution.
But, we'd need to consider MI frontends that enable all-stop+async
somehow, as those wouldn't be expecting such a change...

Thanks.

-------
[PATCH] normal_stop: Extend and clarify comment.

Explain better why we skip saying "Switching to ..." in non-stop mode.

gdb/
2014-03-21  Pedro Alves  <palves@redhat.com>

	* infrun.c (normal_stop): Extend comment.
---
 gdb/ChangeLog |  4 ++++
 gdb/infrun.c  | 16 ++++++++++------
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ba2ca31..0aec7b9 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2014-03-21  Pedro Alves  <palves@redhat.com>
+
+	* infrun.c (normal_stop): Extend comment.
+
 2014-03-21  Hui Zhu  <hui@codesourcery.com>
 	    Pedro Alves  <palves@redhat.com>
 
diff --git a/gdb/infrun.c b/gdb/infrun.c
index d7de7d1..8f9e820 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -6059,18 +6059,22 @@ normal_stop (void)
 	   && last.kind != TARGET_WAITKIND_NO_RESUMED)
     make_cleanup (finish_thread_state_cleanup, &inferior_ptid);
 
-  /* In non-stop mode, we don't want GDB to switch threads behind the
-     user's back, to avoid races where the user is typing a command to
-     apply to thread x, but GDB switches to thread y before the user
-     finishes entering the command.  */
-
   /* As with the notification of thread events, we want to delay
      notifying the user that we've switched thread context until
      the inferior actually stops.
 
      There's no point in saying anything if the inferior has exited.
      Note that SIGNALLED here means "exited with a signal", not
-     "received a signal".  */
+     "received a signal".
+
+     Also skip saying anything in non-stop mode.  In that mode, as we
+     don't want GDB to switch threads behind the user's back, to avoid
+     races where the user is typing a command to apply to thread x,
+     but GDB switches to thread y before the user finishes entering
+     the command, fetch_inferior_event installs a cleanup to restore
+     the current thread back to the thread the user had selected right
+     after this event is handled, so we're not really switching, only
+     informing of a stop.  */
   if (!non_stop
       && !ptid_equal (previous_inferior_ptid, inferior_ptid)
       && target_has_execution
-- 
1.7.11.7