Re: MI3 and async notifications

[Simon Marchi <simark@simark.ca>]

On 2019-06-18 4:38 p.m., Jan Vrany wrote:
> Hi, 
> 
>> I am skeptical about the complex logic you are talking about to handle both
>> =breakpoint-created notifications and responses to the -break-insert.  Both contain pretty
>> much the same information.  So rather than adding an option in GDB for emitting async
>> notifications unconditionally, can't you just process both using the same function?  That
>> doesn't really complicated, but maybe I am misunderstanding your problemi, in which case
>> please expand.
> 
> OK, let me expand (hopefully not too much)
> 
> I have a (general purpose) library that provides higher-level interface
> to GDB/MI. Strictly speaking, it is not bound to any particular UI frontend. 
> 
> This library essentially provides three things: 
> 
> (i) (supposedly) easy to use API to send MI commands, like: 
> 
>     gdb send: (GDBMI_break_insert arguments: { 'main' }) andWithResultDo:[ :result |
>       result isSuccess ifTrue:[
>         Stdout print: 'Breakpoint inserted'
>       ] ifFalse:[
>         Stderr print: 'Oops, something went wrong!'
>       ]
>     ]
>   
>     (sorry for bit arcane syntax, hope you can make sense of it)
> 
> (ii) provide object model of the debugger and its state (like, inferiors, their threads, 
>      frames in thread, variables, registers, breakpoints) like: 
>      
>      "/ Prints the stack of first thread"
>      gdb selectedInferior threads first stack do:[:frame|
>        Stdout print: frame printString
>      ]
> 
>      The idea is that if the inferior is stopped, the model is up-to-date, when it's running,
>      then "reasonable up-to-date", no guarantees.
> 
> (iii) provide a way to get notified of changes, like:
> 
>      gdb when: GDBBreakpointModifiedEvent do: [:event |
>        "/ something has changed, redraw the list to display
>        "/ up-to-date information 
>        breakpointListPane redraw.
>      ]
> 
>      This is essential to build an UI on top of this library
> 
> All the above is API exposed to library user. This design has the advantages
> of being flexible - users can issue commands on their own, I do not have to 
> implement and maintain wrapping API rich enough to handle all cases - and
> and  because is close to GDB/MI, I don't really need to document it in depth, looking to
> GDB documentation gives you very good idea what how to use it. Reusing GDB events
> to notify clients of my library has the same advantage - no need to implement my 
> own event hierarchy and document them. 
> 
> But if I don't get events in cases when they're result of an MI command, the only
> way I can think of handling it is to intercept command result event and:
>  1) examine the result (and sometimes the originating command itself) and do the 
>     processing
>  2) synthesize an event as if it were send by gdb and push it back so it's delivered
>     to users of the library - but in this case I have to make sure it is delivered only
>     to "external" observers and not "internal" observers which are responsible of keeping
>     the model up-to-date. 
>
> Both steps have to cared for case-by-case (like, -break-insert response carries data
> - the breakpoint inserted - while response to -gdb-set is plain ^done so in order to
> update model I have to dig into the command itself, retrieve it's parameters and 
> reconstruct data from there).
> 
> All this is indeed doable and in fact, I do this already for some commands to meet my 
> need back then, but then I realized I need more of this and was thinking how to avoid
> all that code. A quick experiment shows that always emitting a notification solves
> most (all?) issues I experienced, which is why brought it here.
> 
> Does it make sense?

Hi Jan,

Thanks for the detailed explanation.

I am not in your shoes, so I might have a wrong picture of the situation, but this doesn't
sound really complicated.  Isn't "synthesizing" an event from the command result just calling
the same function as you do when getting a =breakpoint-created.  Or am I missing something?

I don't really understand the difference between external and internal observers (that's
probably specific to your design).  Specifically, I don't see what's the difference between
a "real" event that would come from GDB, versus an synthetic event that you would have
injected in your system from the -break-insert response.

Let's say you do get async events for breakpoints you create with -break-insert, then you would
forward these events to all these observers?  So in the case where you generate these events
yourself based on the -break-insert response, why shouldn't you also send them to all observers?
The observers wouldn't know whether it's a "real" event coming from GDB or one you created
yourself, so it shouldn't make any difference to them.

Or (re-reading your message, I realized that this is what you might be trying to explain)
are you saying that your library is very low level, and that users of the library send
"-break-insert" on their own and your library just forwards the MI command to GDB, so your
library doesn't really know (unless it sniffs the user command) that a -break-insert
command has been issued?  If so, that might explain my incomprehension.  All the MI-handling
code I have been exposed to has been a bit more high level, where the user calls some
"breakInsert" function of the library.  The library knows it's sending a -break-insert, so
it can easily handle the response however it wants (including generating a "breakpoint created"
event if it wants to).

>> It would be useful to have a very concrete use case where you could point out "see here,
>> I am missing some info and a notification would be useful".  It could very well be that
>> some notifications are just missing.  
> 
> To make me clear, I'm not saying that some information is missing, just that the way
> it is delivered seem to be inconvenient given the way I use it. It may well be I use it
> the wrong way :-)
We'll see once we understand each other better :).

>> Also, I am a bit worried by a proposal in the thread, which would be to remove information
>> from the -break-insert ^done response, arguing that the async notification would have already
>> been emitted.  It is clear and unambiguous how to map a response to a request, but it would not
>> be obvious to map an async notification to a request.  So it appears to me as a regression in
>> functionality.
> 
> This is why I said that - for example - for -break-insert we need to respond with - at least - 
> ^done,bkpt-number=1. For some other. like -gdb-set I don't think we need to

What I am worried about is that doing a change like this has a pretty big cost for on all frontends
that have this currently implemented, so it needs to have a pretty strong justification.  It's not just
moving the data fields a little bit, or adding something that everybody else can ignore, it would require
a significant flow change.  A frontend that is just interested in setting a breakpoint and getting the
result of that (i.e. the simple case) now needs to do something non-trivial: send the command, listen for
an event, store it somewhere, handle it when receiving the ^done corresponding to the command.  This
brings some concurrency/race condition problems that are just not there in the request-response scheme.

So at least, if we end up choosing to unconditionally emit the =breakpoint-created event, I would prefer
keeping the -break-insert response as-is, for backwards compatibility (for existing frontends) and
simplicity (for basic use cases), even if it means there's some redundancy.

Simon