Re: [RFC] Broken -thread-info output in non-stop mode.

[Vladimir Prus <vladimir@codesourcery.com>]

On Sunday 15 March 2009 21:25:13 Pedro Alves wrote:
> Hi guys,
>
> I've run across this broken MI output in non-stop mode:
>
>  -thread-info
>  =thread-created,id="2",group-id="32"
>  ~"[New Thread 32.32801]\n"
>  ^running
>  *running,thread-id="2"
>  (gdb)
>
> Note that we have output ^running instead of ^done, and, that
> the ",threads=..." part is completelly missing.  I was expecting
> something like this instead:
>
>  -thread-info
>  =thread-created,id="2",group-id="32"
>  *running,thread-id="2"
> 
> ^done,threads=[{id="2",target-id="...",details="...",state="running"},{id="
>1",...,frame={...},state="stopped"}]
>
> -thread-info, is calling target_find_new_threads, and ... that is
> finding new threads.  In non-stop mode, threads we find this way
> have to be running, otherwise we'd have seen a stop reply for
> them already, hence, we call set_running on them.  Then, the problem
> is that we decide to output ^running instead of ^done in the
> mi_on_resume observer.  

Well, there are two problems with the current output.

1. We output ^running as opposed to ^done.
2. When we decide to output ^running, we also discard anything uiout might
contain.

The possible ways to fix (1) are:
    - Special case -thread-info (in mi_on_resume) and output ^done.
	  This is easy. Won't help CLI "info thread" in non-stop mode.
    - Make resume observer distinguish between 'yes, this thread was
      explicitly resumed' and 'we've attached to a thread and found it's
      running'. How hard this would be?
    - Always output ^done, except for a few specially cased MI commands.
      
Speaking of (2), for the sync case it's is mi_on_resume that should print
^running, and if it printed ^running, then there's no way for 
captured_mi_execute_command to print anything while adhering to MI syntax.
So, if ^running was printed mistakenly, we're messed up. For async case,
you are right that ^running can be delayed, and in that case we can print
uiout. However, printing ^running instead of ^done is likely to confuse 
frontends. So, we still need to solve the ^running vs. ^done issue.


> For -thread-info, that should always be "^done", I believe.
>
>  "^done" [ "," results ]
>      The synchronous operation was successful, results are the return
> values.
>
>  "^running"
>      The asynchronous operation was successfully started. The target is
> running.
>
> I kind of only expect ^running on resumption commands (-exec-run,
> -exec-step, -exec-next, etc.).  The docs mention "asynchronous
> operation", although we do output it for synchronous resumptions ...
>
>           ( My interpretation of the docs is that in SYNC mode, -exec-step
>             should output a ^done, when the step is complete, and the
> target stops; while in ASYNC mode, -exec-step would output an immediate
> ^running.  But, that's not how it works... anyway, moving on.  )

The docs use 'async' to mean 'any command that resume the target'. Or,
an alternative interpretation is that the docs are written as if async
mode is the only possible mode ;-)

> Any suggestions on this?  One way I was thinking was to split the
> "this thread is running because we just resumed it"
> from "this thread was already running" set_running calls, and pass
> that information down to the observer, but, I'm not sure that will
> fix it completelly.  In multi-process, we may even find a new
> process when we do a -thread-info, and we may end up doing an internal
> stop/resume bit, thus breaking the output again.

Ouch. But wait, how is 'resume' is done? My impression was that all 
resumptions of the as direct result on user commands are done via
'proceed'. Looking at CVS HEAD, I see one use of proceed in 
proceed_after_attach_callback that I cannot immediately correlate with
a user command, but all others are clearly correlated. So, maybe MI
should output ^running only when proceed is called?

> My next bet is to still do what the patch below does, but in addition,
> never output "^running" if the command was a "-thread-info", similarly
> to the hack in place that checks for "target-select".  Or the other way
> around, white list which commands want ^running instead of ^done --- AFAIK,
> only the resumption commands are expected to output ^running: -exec-run,
> -exec-step, -exec-next, etc.  For MI3, when we completelly get rid of
> "^running", the hack could go away.  This would leave CLI commands issued
> from MI to solve --- "info threads" would still output ^running...
> It is a problem?  I'm guessing it would be, but I'm not sure.
>
> The patch at:
>
>  http://sourceware.org/ml/gdb-patches/2008-06/msg00247.html
>
> ... claimed that it is a bonus.  Is it really a bonus, in that would
> any frontend really expect it?  Given that CLI commands didn't output
> ^running for close to 10 years, how bad would it be if we reverted
> back to not doing it again, at least for async targets?

Well, they did not output ^running for 10 years, but it does not mean
it was bad. KDevelop users did complain that issuing CLI commands resulted
in no feedback that the application is actually running. We can suppress
this in async mode, but then the solution starts to include so many 
assumptions that it makes me nervous ;-) Let me know what you think about
the proceed idea.

- Volodya


>
> Any other suggestions?