Re: MI3 and async notifications

Jan Vrany
2019-06-19T20:58:06Z

Hi,

On Wed, 2019-06-19 at 11:29 -0400, Simon Marchi wrote:
>
>
> 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?

In case of -break-insert is not that much of work since command response carries the breakpoint
object and event also arries just the breakpoint objects.

But consider `set` CLI and `-gdb-set` MI commands:

(gdb)
set directories /foo/bar
&"set directories /foo/bar\n"
&"Warning: /foo/bar: No such file or directory.\n"
=cmd-param-changed,param="directories",value="/foo/bar:$cdir:$cwd"
^done
(gdb)

and

-gdb-set directories /baz/qux
&"Warning: /baz/qux: No such file or directory.\n"
^done

In this case. the response carries no data at all so in order to synthesize the event
I need to look at the originating command and parse its arguments to pick the name and value.
And as you can see above, the value parameter to the command is just "/foo/bar" while
the value in the event is "/foo/bar:$cdir:$cwd" - another little thing to care about in this
very case.

Indeed, not a rocket science, but as you see, it requires to write a little specialized
piece of code for "every" command (vs. not having to write anything at all if I'd get the event from
GDB). Maybe the phrase "complex logic" was improper - complex in a way that each case has to be
dealt with individually and if one forgets, the internal model can get out of sync.

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

Yeah, this is specific to my design. It is just that the library itself registers observers
on events in order to update the model of debugger (using the same mechanic, same patterns)
so when it is "synthetic" event, is has to be processed slightly differently. Not sure if it makes
much sense to go to such details.

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

Precisely! The library does not really know and therefore would have to "sniff".

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

Yeah, I guess that's more common approach. I decided to make it more lower-level as this gives me
more flexibility. Or so it looked back then :-)

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

Yes, I completely understand. This and this is a valid concern. I said it before - the last thing I want is to
break other people's stuff "that works for decades" because of my obscure thing. If the decision out of this
discussion is "no", I'll happily deal with it in my library. No problem whatsoever.

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

Yes, that was my original proposal, keep everything as it is, just to emit events in addition.
Either unconditionally or only if mi-always-async is set (at the cost of having yet another option
which is also far from ideal).

Jan