Re: No thread context switch in non-stop mode???

[Doug Evans <dje at google dot com>]

Pedro Alves writes:
 > On 03/16/2014 02:12 AM, Doug Evans wrote:
 > > Hi.
 > > 
 > > I'm looking into implementing a wait command, and have come across
 > > something I don't understand.
 > > 
 > > I understand the goal of this comment in infrun.c:
 > > 
 > >   /* 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.  */
 > > 
 > > Sounds reasonable to me.
 > > But I'm finding (apparent) violations of this and wondering if they're all bugs.
 > > 
 > > E.g.  With the appended testcase I see gdb changing the current thread
 > > each time a breakpoint is hit, despite the above claim.
 > 
 > That code, and that the comment is talking about, is the case of the
 > current thread seen by the user that is typing a command.

Right.  That's understood!  :-)

 >  IOW, the
 > current thread seen by the top level interpreter that reacts to
 > input (or really, whatever nests an event loop).
 > 
 > Your test:
 > 
 >  b woke_up
 >  commands
 >  i thr
 >  end
 > 
 > is printing the current thread while the breakpoint command is
 > active.  That's temporarily set to the thread that reported the
 > event, so the commands apply in context of the thread that got
 > the event.  I think that's what makes most sense.

Ah, righto.
I saw an instance of the current thread changing on me while just sitting
at the prompt (in non-stop mode), and I was trying to come up with a repro.
Must have been mistaken - I was playing with all variations of
target_async + non-stop, and probably lost track of which variation
I was using.  Bleah.

 > > 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.

 > If it weren't for the cleanup,

For clarity's sake, which cleanup?

 > all the context_switch calls
 > switch_to_thread calls in handle_inferior_event would change the
 > selected thread behind the typing-user's back.  So what happens is:
 > 
 > - user has thread A selected, and may be typing command for it.
 >   Could be "kill" even -- best not switch focus to another thread
 >   behind the user's back and kill the wrong process!
 > - a target event triggers (for thread B).
 > - even loop ends up calling into fetch_inferior_event.
 > - fetch_inferior_event saves the user's current thread.
 > - handle_inferior_event handles the event.  It was an event
 >   for thread B.  switch_to_thread(B) is called somewhere down
 >   handle_inferior_event's leaf functions.
 > - handle_inferior_event returns (with B selected as current).
 >   If this was a breakpoint with a command attached, the breakpoint's
 >   command is ran with M selected as current (from within
 >   inferior_event_handler).
 > - fetch_inferior_event restores the user selected thread.
 > 
 > > Plus, gdb prints a breakpoint hit
 > > notification but without also printing which thread hit the breakpoint
 > > (even if the current thread isn't changed), the message is less useful
 > > than it could be.
 > 
 > Yes, agreed.  Last time I tried to change things in this direction
 > I got some backlash:
 > 
 >  https://www.sourceware.org/ml/gdb/2012-11/msg00036.html
 > 
 > That discussion and more recent developments around go green threads,
 > light weight execution units, fibers, etc., make me a little more
 > inclined towards John's view though, though yeah, we should definitely
 > say which thread got the event if there are threads around.

I don't want to conflate the issue I'm raising with that discussion.
One can certainly debate what the "%s" should be in "[Switching to %s]\n"
in a [sic] non-threaded case, but I can't see a good case to be made
against printing the thread (when different from the current thread)
when gdb is already printing:

Breakpoint 2, woke_up () at test.c:38
38	}