* MI3 and async notifications @ 2019-06-10 21:19 Jan Vrany 2019-06-10 23:23 ` Jonah Graham ` (2 more replies) 0 siblings, 3 replies; 26+ messages in thread From: Jan Vrany @ 2019-06-10 21:19 UTC (permalink / raw) To: gdb Hello, as an user og GDB/MI (and frontent developer), I'd like to open a discussion about one aspect of MI that I'd like to change in MI3 before it is released into the wild. Currently, quite a few commands suppress async notifications when a command is issued through MI. For example, when a breakpoint is added using -break-insert then =breakpoint-created is not emitted. At least in my case, this behavior complicates client design because now the client has to care for new breakpoints on multiple places: (i) listen to breakpoint created events to handle cases where breakpoint is added via CLI. (ii) do similar processing whenever MI returns ^done for previously issued -break-insert command. This leads to a complex logic, especially in cases where frontend has some scripting support (so execution of MI commands is not completely under frontend developer's control). Emitting notifications unconditionally would simplify things a lot - again at least in my case. Therefore I'd like to propose a change for MI3 to always send notifications. If such a change would make things complicated for other frontends (Eclipse CDT / Emacs come to mind), I propose new -gdb-set mi-always-notify 1 -gdb-set mi-always-notify 0 setting to control the behavior so frontends may choose. I'm happy to submit a patch. Are there any other frontend developers? If so, would it be OK to always notify in MI3 or do you prefer to have an option? Any other comments? Thanks! Jan ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-10 21:19 MI3 and async notifications Jan Vrany @ 2019-06-10 23:23 ` Jonah Graham 2019-06-11 8:50 ` Jan Vrany 2019-06-15 14:34 ` Tom Tromey 2019-06-18 3:14 ` Simon Marchi 2 siblings, 1 reply; 26+ messages in thread From: Jonah Graham @ 2019-06-10 23:23 UTC (permalink / raw) To: Jan Vrany; +Cc: gdb On Mon, 10 Jun 2019 at 17:19, Jan Vrany <jan.vrany@fit.cvut.cz> wrote: > Therefore I'd like to propose a change for MI3 to always send > notifications. > If such a change would make things complicated for other frontends > (Eclipse CDT / Emacs come to mind), I propose new > > -gdb-set mi-always-notify 1 > -gdb-set mi-always-notify 0 > Thank you for considering the other front end consumers of MI. I am one of the current maintainers of CDT so I will share my 2cents. Eclipse CDT would certainly require such a flag, but only if MI3 was a replacement for MI2. If CDT can continue to use gdb in mi2 mode (CDT launches gdb with --interpreter mi2 [1]) then I don't think you need to carry on the extra logic in MI3. I haven't followed the discussions on MI3 closely. I assume from the proposal that the -break-insert still gets the done message with the breakpoint number in it? And does the async message come back after the the ^done? If it does not come after the done CDT will have to hold processing the async message until after it finds out if the =breakpoint-created was for the MI or CLI inserted breakpoint (consider the race condition that a user / script inserts a breakpoint from the CLI at the same time as from the MI). I hope that helps from CDT perspective. Jonah [1] https://github.com/eclipse-cdt/cdt/blob/7741bd98f7b08a281c4b7f60e60c5839f315f760/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBBackend.java#L188 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-10 23:23 ` Jonah Graham @ 2019-06-11 8:50 ` Jan Vrany 2019-06-11 13:37 ` Jonah Graham 0 siblings, 1 reply; 26+ messages in thread From: Jan Vrany @ 2019-06-11 8:50 UTC (permalink / raw) To: gdb; +Cc: Jonah Graham On Mon, 2019-06-10 at 19:22 -0400, Jonah Graham wrote: > On Mon, 10 Jun 2019 at 17:19, Jan Vrany <jan.vrany@fit.cvut.cz> wrote: > > > Therefore I'd like to propose a change for MI3 to always send > > notifications. > > If such a change would make things complicated for other frontends > > (Eclipse CDT / Emacs come to mind), I propose new > > > > -gdb-set mi-always-notify 1 > > -gdb-set mi-always-notify 0 > > > > Thank you for considering the other front end consumers of MI. I am one of > the current maintainers of CDT so I will share my 2cents. Thanks! > > Eclipse CDT would certainly require such a flag, but only if MI3 was a > replacement for MI2. If CDT can continue to use gdb in mi2 mode (CDT > launches gdb with --interpreter mi2 [1]) then I don't think you need to > carry on the extra logic in MI3. I haven't followed the discussions on MI3 > closely. I don't know what are the plans exactly either. My idea was to change it for MI3 only, so if you do `--interpreter mi2`, nothing will change for you. However I guess you may want to use MI3 in future as it will be the default (I might be wrong but as of current master, MI3 is the default). Not breaking CDT is a (very) strong argument, so I think we need the option. It's not that hard to implement. > > I assume from the proposal that the -break-insert still gets the done > message with the breakpoint number in it? And does the async message come > back after the the ^done? If it does not come after the done CDT will have > to hold processing the async message until after it finds out if the > =breakpoint-created was for the MI or CLI inserted breakpoint Yes, you will get ^done with all the data as before, only that before ^done you'll get =breakpoint-created event (with the same data). I modified the code to always send notifications in MI3 but not MI2 (patch below), here's an example session to demonstrate the difference on -break-insert: MI2 (current) ./gdb -i=mi2 ./testsuite/outputs/gdb.python/py-msymbol/py-msymbol =thread-group-added,id="i1" ~"GNU gdb (GDB) 8.3.50.20190611-git\n" ~"Copyright (C) 2019 Free Software Foundation, Inc.\n" ~"License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>\nThis is free software: you are free to change and redistribute it.\nThere is NO WARRANTY, to the extent permitted by law." ~"\nType \"show copying\" and \"show warranty\" for details.\n" ~"This GDB was configured as \"x86_64-pc-linux-gnu\".\n" ~"Type \"show configuration\" for configuration details.\n" ~"For bug reporting instructions, please see:\n" ~"<http://www.gnu.org/software/gdb/bugs/>.\n" ~"Find the GDB manual and other documentation resources online at:\n <http://www.gnu.org/software/gdb/documentation/>;." ~"\n\n" ~"For help, type \"help\".\n" ~"Type \"apropos word\" to search for commands related to \"word\"...\n" ~"Reading symbols from ./testsuite/outputs/gdb.python/py-msymbol/py-msymbol...\n" (gdb) -break-insert main ^done,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x0000000000001151",func="main",file="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.python/py- symbol.c",fullname="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.python/py-symbol.c",line="56",thread-groups=["i1"],times="0",original-location="main"} (gdb) quit &"quit\n" MI3 with modification (proposed) jv@sao:~/Projects/gdb/users_jv_patches/gdb$ ./gdb -i=mi3 ./testsuite/outputs/gdb.python/py-msymbol/py-msymbol =thread-group-added,id="i1" ~"GNU gdb (GDB) 8.3.50.20190611-git\n" ~"Copyright (C) 2019 Free Software Foundation, Inc.\n" ~"License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>\nThis is free software: you are free to change and redistribute it.\nThere is NO WARRANTY, to the extent permitted by law." ~"\nType \"show copying\" and \"show warranty\" for details.\n" ~"This GDB was configured as \"x86_64-pc-linux-gnu\".\n" ~"Type \"show configuration\" for configuration details.\n" ~"For bug reporting instructions, please see:\n" ~"<http://www.gnu.org/software/gdb/bugs/>.\n" ~"Find the GDB manual and other documentation resources online at:\n <http://www.gnu.org/software/gdb/documentation/>;." ~"\n\n" ~"For help, type \"help\".\n" ~"Type \"apropos word\" to search for commands related to \"word\"...\n" ~"Reading symbols from ./testsuite/outputs/gdb.python/py-msymbol/py-msymbol...\n" (gdb) -break-insert main =breakpoint-created,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x0000000000001151",func="main",file="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.python/py- symbol.c",fullname="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.python/py-symbol.c",line="56",thread-groups=["i1"],times="0",original-location="main"} ^done,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x0000000000001151",func="main",file="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.python/py- symbol.c",fullname="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.python/py-symbol.c",line="56",thread-groups=["i1"],times="0",original-location="main"} (gdb) quit &"quit\n" As you can see, you get the notification *before* ^done response. Does that answer your questions? > (consider the > race condition that a user / script inserts a breakpoint from the CLI at > the same time as from the MI). I see. > > I hope that helps from CDT perspective. > Sure, thanks! So it looks me a new option is the way to go. Jan -- diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 13c310d494..2f2d1f2612 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -1954,7 +1954,8 @@ mi_execute_command (const char *cmd, int from_tty) gdb::optional<scoped_restore_tmpl<int>> restore_suppress; - if (command->cmd != NULL && command->cmd->suppress_notification != NULL) + if (command->cmd != NULL && command->cmd->suppress_notification != NULL + && mi_version(current_uiout) < 3) restore_suppress.emplace (command->cmd->suppress_notification, 1); command->token = token; ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-11 8:50 ` Jan Vrany @ 2019-06-11 13:37 ` Jonah Graham 2019-07-05 20:00 ` Pedro Alves 0 siblings, 1 reply; 26+ messages in thread From: Jonah Graham @ 2019-06-11 13:37 UTC (permalink / raw) To: Jan Vrany; +Cc: gdb On Tue, 11 Jun 2019 at 04:50, Jan Vrany <jan.vrany@fit.cvut.cz> wrote: > As you can see, you get the notification *before* ^done response. Does > that answer > your questions? > > Yes that does. I think CDT will use the mi-always-notify option. Jonah ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-11 13:37 ` Jonah Graham @ 2019-07-05 20:00 ` Pedro Alves 2019-07-05 21:58 ` Jonah Graham 0 siblings, 1 reply; 26+ messages in thread From: Pedro Alves @ 2019-07-05 20:00 UTC (permalink / raw) To: Jonah Graham, Jan Vrany; +Cc: gdb On 6/11/19 2:37 PM, Jonah Graham wrote: > On Tue, 11 Jun 2019 at 04:50, Jan Vrany <jan.vrany@fit.cvut.cz> wrote: > >> As you can see, you get the notification *before* ^done response. Does >> that answer >> your questions? >> >> > Yes that does. I think CDT will use the mi-always-notify option. Funny. ISTR that the breakpoint notification suppressing was originally done for CDT and reading/hearing Vladimir argue for it. :-D Though I can't find any arguing about that aspect around the original patch now: https://sourceware.org/ml/gdb-patches/2011-04/msg00471.html Vladimir said "As other MI notifications", but I checked out the tree for that commit (8d3788bd590a), and I couldn't find any other suppression that existed back then. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-07-05 20:00 ` Pedro Alves @ 2019-07-05 21:58 ` Jonah Graham 0 siblings, 0 replies; 26+ messages in thread From: Jonah Graham @ 2019-07-05 21:58 UTC (permalink / raw) To: Pedro Alves; +Cc: Jan Vrany, gdb On Fri, 5 Jul 2019 at 16:00, Pedro Alves <palves@redhat.com> wrote: > On 6/11/19 2:37 PM, Jonah Graham wrote: > > On Tue, 11 Jun 2019 at 04:50, Jan Vrany <jan.vrany@fit.cvut.cz> wrote: > > > >> As you can see, you get the notification *before* ^done response. Does > >> that answer > >> your questions? > >> > >> > > Yes that does. I think CDT will use the mi-always-notify option. > > Funny. ISTR that the breakpoint notification suppressing was > originally done for CDT and reading/hearing Vladimir argue for > it. :-D Though I can't find any arguing about that aspect > around the original patch now: > > https://sourceware.org/ml/gdb-patches/2011-04/msg00471.html > > Vladimir said "As other MI notifications", but I checked out > the tree for that commit (8d3788bd590a), and I couldn't find > any other suppression that existed back then. > To be clear, "CDT will use the mi-always-notify" meant that CDT would make use of the option to turn the always notify *off*. In other words, I wanted GDB to provide it as an option and not have it always on. Jonah ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-10 21:19 MI3 and async notifications Jan Vrany 2019-06-10 23:23 ` Jonah Graham @ 2019-06-15 14:34 ` Tom Tromey 2019-06-17 10:53 ` Jan Vrany 2019-06-18 3:14 ` Simon Marchi 2 siblings, 1 reply; 26+ messages in thread From: Tom Tromey @ 2019-06-15 14:34 UTC (permalink / raw) To: Jan Vrany; +Cc: gdb >>>>> "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes: Jan> as an user og GDB/MI (and frontent developer), I'd like to Jan> open a discussion about one aspect of MI that I'd like to change Jan> in MI3 before it is released into the wild. ... Jan> Emitting notifications unconditionally would simplify things a lot Jan> - again at least in my case. It seems like a good idea to me. I wonder if it makes sense to go even further and say there will only be async notifications for things like this. Tom ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-15 14:34 ` Tom Tromey @ 2019-06-17 10:53 ` Jan Vrany 2019-06-17 12:11 ` Jonah Graham ` (2 more replies) 0 siblings, 3 replies; 26+ messages in thread From: Jan Vrany @ 2019-06-17 10:53 UTC (permalink / raw) To: Tom Tromey, Jonah Graham; +Cc: gdb On Sat, 2019-06-15 at 08:34 -0600, Tom Tromey wrote: > > > > > > "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes: > > Jan> as an user og GDB/MI (and frontent developer), I'd like to > Jan> open a discussion about one aspect of MI that I'd like to change > Jan> in MI3 before it is released into the wild. > ... > Jan> Emitting notifications unconditionally would simplify things a lot > Jan> - again at least in my case. > > It seems like a good idea to me. I wonder if it makes sense to go even > further and say there will only be async notifications for things like > this. Yes, I thought the same initially. But then what about other existing MI consumers? From what I understood from Jonah's comments earlier, this would break (at least) CDT. So CDT would either have to stick with MI2 (not great in a long term) or refactor their code (not sure CDT guys would be happy to do so, especially as - I presume - CDT needs to support wide range of GDB versions already in the wild, a problem I do not have). While I personally agree with you and will be happy to go that far, it'd break existing consumers - something that should IMO be carefully discussed and planned. Adding a new option as I proposed as an alternative will be backward compatible, indeed at the cost of more convoluted code in GDB itself. Is anyone from Emacs community around? Or any other MI consumers? Jan > > Tom ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-17 10:53 ` Jan Vrany @ 2019-06-17 12:11 ` Jonah Graham 2019-06-17 12:14 ` Joel Brobecker 2019-06-17 19:52 ` André Pönitz 2 siblings, 0 replies; 26+ messages in thread From: Jonah Graham @ 2019-06-17 12:11 UTC (permalink / raw) To: Jan Vrany; +Cc: Tom Tromey, gdb On Mon, 17 Jun 2019 at 06:53, Jan Vrany <jan.vrany@fit.cvut.cz> wrote: > On Sat, 2019-06-15 at 08:34 -0600, Tom Tromey wrote: > > From what I understood from Jonah's comments earlier, this would break (at > least) > CDT. So CDT would either have to stick with MI2 (not great in a long term) > or refactor their code (not sure CDT guys would be happy to do so, > especially as > - I presume - CDT needs to support wide range of GDB versions already in > the wild, > a problem I do not have). > Yes, you have understood correctly. CDT currently supports GDB 7.5+. Jonah ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-17 10:53 ` Jan Vrany 2019-06-17 12:11 ` Jonah Graham @ 2019-06-17 12:14 ` Joel Brobecker 2019-06-17 12:26 ` Jonah Graham 2019-06-17 19:52 ` André Pönitz 2 siblings, 1 reply; 26+ messages in thread From: Joel Brobecker @ 2019-06-17 12:14 UTC (permalink / raw) To: Jan Vrany; +Cc: Tom Tromey, Jonah Graham, gdb > > It seems like a good idea to me. I wonder if it makes sense to go even > > further and say there will only be async notifications for things like > > this. > > Yes, I thought the same initially. But then what about other existing MI > consumers? > > >From what I understood from Jonah's comments earlier, this would break (at least) > CDT. So CDT would either have to stick with MI2 (not great in a long > term) or refactor their code (not sure CDT guys would be happy to do > so, especially as - I presume - CDT needs to support wide range of GDB > versions already in the wild, a problem I do not have). > > While I personally agree with you and will be happy to go that far, > it'd break existing consumers - something that should IMO be carefully > discussed and planned. > > Adding a new option as I proposed as an alternative will be backward > compatible, indeed at the cost of more convoluted code in GDB itself. > > Is anyone from Emacs community around? Or any other MI consumers? My 2 cents: In my opinion, while I think upward compatibility is very important, it is also important to avoid having too many configurability options. Otherwise, we end up with a large number of options and the testing matrix, if we want to verify that they work well together, quickly explodes. In this case, because we have MI versions, and because the notification shouldn't be different from the data of the "^done" message, I think the incompatibility would be acceptable -- assuming existing parsers don't come back to say that it actually is a large effort for them to adapt. -- Joel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-17 12:14 ` Joel Brobecker @ 2019-06-17 12:26 ` Jonah Graham 2019-06-17 12:56 ` Joel Brobecker 0 siblings, 1 reply; 26+ messages in thread From: Jonah Graham @ 2019-06-17 12:26 UTC (permalink / raw) To: Joel Brobecker; +Cc: Jan Vrany, Tom Tromey, gdb On Mon, 17 Jun 2019 at 08:14, Joel Brobecker <brobecker@adacore.com> wrote: > > > It seems like a good idea to me. I wonder if it makes sense to go even > > > further and say there will only be async notifications for things like > > > this. > > > > Yes, I thought the same initially. But then what about other existing MI > > consumers? > > In my opinion, while I think upward compatibility is very important, > it is also important to avoid having too many configurability options. > Otherwise, we end up with a large number of options and the testing > matrix, if we want to verify that they work well together, quickly > explodes. > > In this case, because we have MI versions, and because the notification > shouldn't be different from the data of the "^done" message, I think > the incompatibility would be acceptable -- assuming existing parsers > don't come back to say that it actually is a large effort for them > to adapt. > > I do agree, avoid the extra configurability - but I simply don't know how to work with just async notifications to sync messages. It means that CDT will have to issues the -break-insert, look for the done message and "search" between them to find the =breakpoint-created that matched and separately process any that don't. Please see my earlier message about how to handle race condition between -break-inserts over MI and breaks inserted from CLI. This race condition does not happen during normal operation (where a human is driving everything) but does kick in during many semi-automated flows. Perhaps this isn't a big problem, but to me it seems the logic to match up -break-insert to =breakpoint-created in client side is complex and bug prone. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-17 12:26 ` Jonah Graham @ 2019-06-17 12:56 ` Joel Brobecker 2019-06-17 13:12 ` Jan Vrany 2019-06-17 13:12 ` Jonah Graham 0 siblings, 2 replies; 26+ messages in thread From: Joel Brobecker @ 2019-06-17 12:56 UTC (permalink / raw) To: Jonah Graham; +Cc: Jan Vrany, Tom Tromey, gdb > I do agree, avoid the extra configurability - but I simply don't know how > to work with just async notifications to sync messages. It means that CDT > will have to issues the -break-insert, look for the done message and > "search" between them to find the =breakpoint-created that matched and > separately process any that don't. Please see my earlier message about how > to handle race condition between -break-inserts over MI and breaks inserted > from CLI. This race condition does not happen during normal operation > (where a human is driving everything) but does kick in during many > semi-automated flows. Perhaps this isn't a big problem, but to me it seems > the logic to match up -break-insert to =breakpoint-created in client side > is complex and bug prone. The part I don't understand is why it matters to sync the two. -- Joel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-17 12:56 ` Joel Brobecker @ 2019-06-17 13:12 ` Jan Vrany 2019-06-17 13:23 ` Jonah Graham 2019-06-17 13:12 ` Jonah Graham 1 sibling, 1 reply; 26+ messages in thread From: Jan Vrany @ 2019-06-17 13:12 UTC (permalink / raw) To: Joel Brobecker, Jonah Graham; +Cc: Tom Tromey, gdb On Mon, 2019-06-17 at 08:56 -0400, Joel Brobecker wrote: > > I do agree, avoid the extra configurability - but I simply don't know how > > to work with just async notifications to sync messages. It means that CDT > > will have to issues the -break-insert, look for the done message and > > "search" between them to find the =breakpoint-created that matched and > > separately process any that don't. Please see my earlier message about how > > to handle race condition between -break-inserts over MI and breaks inserted > > from CLI. This race condition does not happen during normal operation > > (where a human is driving everything) but does kick in during many > > semi-automated flows. Perhaps this isn't a big problem, but to me it seems > > the logic to match up -break-insert to =breakpoint-created in client side > > is complex and bug prone. > > The part I don't understand is why it matters to sync the two. > Jonah, I was about to ask the same. I understand that you need to know which breakpoint has been inserted by given command, but this if we respond with something like 1-break-insert main =breakpoint-created,bkpt={number="1",type=...} 1^done,bkpt-number=1 then you just search for breakpoint with that id, no? Given that MI guarantees that =breakpoint-created arrives before ^done reply to command. Am I missing something? Also note that this is not only about breakpoints but also about things like -gdb-set and few others. Jan ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-17 13:12 ` Jan Vrany @ 2019-06-17 13:23 ` Jonah Graham 2019-06-17 20:45 ` Joel Brobecker 0 siblings, 1 reply; 26+ messages in thread From: Jonah Graham @ 2019-06-17 13:23 UTC (permalink / raw) To: Jan Vrany; +Cc: Joel Brobecker, Tom Tromey, gdb On Mon, 17 Jun 2019 at 09:12, Jan Vrany <jan.vrany@fit.cvut.cz> wrote: > On Mon, 2019-06-17 at 08:56 -0400, Joel Brobecker wrote: > > > I do agree, avoid the extra configurability - but I simply don't know > how > > > to work with just async notifications to sync messages. It means that > CDT > > > will have to issues the -break-insert, look for the done message and > > > "search" between them to find the =breakpoint-created that matched and > > > separately process any that don't. Please see my earlier message about > how > > > to handle race condition between -break-inserts over MI and breaks > inserted > > > from CLI. This race condition does not happen during normal operation > > > (where a human is driving everything) but does kick in during many > > > semi-automated flows. Perhaps this isn't a big problem, but to me it > seems > > > the logic to match up -break-insert to =breakpoint-created in client > side > > > is complex and bug prone. > > > > The part I don't understand is why it matters to sync the two. > > > Jonah, I was about to ask the same. I understand that you need to know > which breakpoint has been inserted by given command, but this > if we respond with something like > > 1-break-insert main > =breakpoint-created,bkpt={number="1",type=...} > 1^done,bkpt-number=1 > > then you just search for breakpoint with that id, no? Given that MI > guarantees that =breakpoint-created arrives before ^done reply to command. > Am I missing something? > No you aren't missing anything. That would be a perfectly acceptable solution for CDT. There would still be some other new logic needed for CDT, we would still have to store all the =breakpoint-created if there is a -break-insert active and then process all of them when the ^done is received. However that seems fairly reasonable. > > Also note that this is not only about breakpoints but also about things > like > -gdb-set and few others. > ?? This is down to me not following closely enough. If you can point me in the right direction then I can provide feedback now. If not it will have to wait until if/when CDT needs to update to MI3 support. However there are no gdb-sets that are likely to have any such issues as there is little to no UI elements connected to gdb-sets. CDT does all of its gdb-sets at startup to set the environment and then does not normally change any of them. Thanks, Jonah ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-17 13:23 ` Jonah Graham @ 2019-06-17 20:45 ` Joel Brobecker 2019-06-17 20:58 ` Jan Vrany 0 siblings, 1 reply; 26+ messages in thread From: Joel Brobecker @ 2019-06-17 20:45 UTC (permalink / raw) To: Jonah Graham; +Cc: Jan Vrany, Tom Tromey, gdb > > Jonah, I was about to ask the same. I understand that you need to know > > which breakpoint has been inserted by given command, but this > > if we respond with something like > > > > 1-break-insert main > > =breakpoint-created,bkpt={number="1",type=...} > > 1^done,bkpt-number=1 > > > > then you just search for breakpoint with that id, no? Given that MI > > guarantees that =breakpoint-created arrives before ^done reply to command. > > Am I missing something? > > > > No you aren't missing anything. That would be a perfectly acceptable > solution for CDT. > > There would still be some other new logic needed for CDT, we would > still have to store all the =breakpoint-created if there is a > -break-insert active and then process all of them when the ^done is > received. However that seems fairly reasonable. Do we even need the bkpt-number=1 attribute in the "done" command? The notification includes that information, so the GUI should have enough info from there to determine which UI element to update, right? -- Joel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-17 20:45 ` Joel Brobecker @ 2019-06-17 20:58 ` Jan Vrany 2019-06-17 21:50 ` Jonah Graham 0 siblings, 1 reply; 26+ messages in thread From: Jan Vrany @ 2019-06-17 20:58 UTC (permalink / raw) To: Joel Brobecker, Jonah Graham; +Cc: Tom Tromey, gdb, André Pönitz On Mon, 2019-06-17 at 16:45 -0400, Joel Brobecker wrote: > > > Jonah, I was about to ask the same. I understand that you need to know > > > which breakpoint has been inserted by given command, but this > > > if we respond with something like > > > > > > 1-break-insert main > > > =breakpoint-created,bkpt={number="1",type=...} > > > 1^done,bkpt-number=1 > > > > > > then you just search for breakpoint with that id, no? Given that MI > > > guarantees that =breakpoint-created arrives before ^done reply to command. > > > Am I missing something? > > > > > > > No you aren't missing anything. That would be a perfectly acceptable > > solution for CDT. > > > > There would still be some other new logic needed for CDT, we would > > still have to store all the =breakpoint-created if there is a > > -break-insert active and then process all of them when the ^done is > > received. However that seems fairly reasonable. > > Do we even need the bkpt-number=1 attribute in the "done" command? > The notification includes that information, so the GUI should have > enough info from there to determine which UI element to update, right? > I think so. Imagine you have a UI with breakpoint list and there's an "Add Breakpoint" button. When "Add Breakpoint" is pressed, user fills location, press "OK", new breakpoint is added and *UI pre-selects just added breakpoint*. This is IMO sensible UI behavior. Now, when you press "OK" to add the breakpoint on a location, behind the scenes frontend issues -break-insert command and waits for ^done confirmation. But as Jonah mentioned, another breakpoint could be inserted meanwhile so you'd get something like this on MI channel. 10-break-insert main =breakpoint-created,bkpt={number="1",type=...<some unrelated breakpoint from CLI or script>} =breakpoint-created,bkpt={number="2",type=...<the breakpoint on main you just requested>} =breakpoint-created,bkpt={number="3",type=...<some unrelated breakpoint from CLI or script>} 10^done Now, if ^done response would not contain bkpt-number=2 attribute, how would you know which is the breakpoint you need to pre-select in the UI? Jan ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-17 20:58 ` Jan Vrany @ 2019-06-17 21:50 ` Jonah Graham 0 siblings, 0 replies; 26+ messages in thread From: Jonah Graham @ 2019-06-17 21:50 UTC (permalink / raw) To: Jan Vrany; +Cc: Joel Brobecker, Tom Tromey, gdb, André Pönitz On Mon, 17 Jun 2019 at 16:58, Jan Vrany <jan.vrany@fit.cvut.cz> wrote: > On Mon, 2019-06-17 at 16:45 -0400, Joel Brobecker wrote: > > > > Jonah, I was about to ask the same. I understand that you need to > know > > > > which breakpoint has been inserted by given command, but this > > > > if we respond with something like > > > > > > > > 1-break-insert main > > > > =breakpoint-created,bkpt={number="1",type=...} > > > > 1^done,bkpt-number=1 > > > > > > > > then you just search for breakpoint with that id, no? Given that MI > > > > guarantees that =breakpoint-created arrives before ^done reply to > command. > > > > Am I missing something? > > > > > > > > > > No you aren't missing anything. That would be a perfectly acceptable > > > solution for CDT. > > > > > > There would still be some other new logic needed for CDT, we would > > > still have to store all the =breakpoint-created if there is a > > > -break-insert active and then process all of them when the ^done is > > > received. However that seems fairly reasonable. > > > > Do we even need the bkpt-number=1 attribute in the "done" command? > > The notification includes that information, so the GUI should have > > enough info from there to determine which UI element to update, right? > > > > I think so. Imagine you have a UI with breakpoint list and there's > an "Add Breakpoint" button. When "Add Breakpoint" is pressed, user > fills location, press "OK", new breakpoint is added and > *UI pre-selects just added breakpoint*. This is IMO sensible UI behavior. > > Now, when you press "OK" to add the breakpoint on a location, behind the > scenes > frontend issues -break-insert command and waits for ^done confirmation. > But as Jonah mentioned, another breakpoint could be inserted meanwhile so > you'd get something like this on MI channel. > > 10-break-insert main > =breakpoint-created,bkpt={number="1",type=...<some unrelated breakpoint > from CLI or script>} > =breakpoint-created,bkpt={number="2",type=...<the breakpoint on main you > just requested>} > =breakpoint-created,bkpt={number="3",type=...<some unrelated breakpoint > from CLI or script>} > 10^done > > Now, if ^done response would not contain bkpt-number=2 attribute, how > would you know which > is the breakpoint you need to pre-select in the UI? > > > Thanks Jan, that looks like a good example to show the problem. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-17 12:56 ` Joel Brobecker 2019-06-17 13:12 ` Jan Vrany @ 2019-06-17 13:12 ` Jonah Graham 1 sibling, 0 replies; 26+ messages in thread From: Jonah Graham @ 2019-06-17 13:12 UTC (permalink / raw) To: Joel Brobecker; +Cc: Jan Vrany, Tom Tromey, gdb On Mon, 17 Jun 2019 at 08:56, Joel Brobecker <brobecker@adacore.com> wrote: > The part I don't understand is why it matters to sync the two. > For example, to allow program to update the UI element that represents a breakpoint. In CDT when a breakpoint is inserted in the UI and fails to be installed or is only pending the UI element is updated to display that fact. CDT maintains a map of UI elements to GDB instances+breakpoint numbers. That connection (UI element to breakpoint number) is made as result of the response to -break-insert. The UI elements are breakpoints in a table, or in the margin of an editor. Does that make sense? ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-17 10:53 ` Jan Vrany 2019-06-17 12:11 ` Jonah Graham 2019-06-17 12:14 ` Joel Brobecker @ 2019-06-17 19:52 ` André Pönitz 2 siblings, 0 replies; 26+ messages in thread From: André Pönitz @ 2019-06-17 19:52 UTC (permalink / raw) To: Jan Vrany; +Cc: Tom Tromey, Jonah Graham, gdb On Mon, Jun 17, 2019 at 11:53:14AM +0100, Jan Vrany wrote: > On Sat, 2019-06-15 at 08:34 -0600, Tom Tromey wrote: > > > > > > > "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes: > > > > Jan> as an user og GDB/MI (and frontent developer), I'd like to Jan> > > open a discussion about one aspect of MI that I'd like to change Jan> in > > MI3 before it is released into the wild. ... Jan> Emitting > > notifications unconditionally would simplify things a lot Jan> - again > > at least in my case. > > > > It seems like a good idea to me. I wonder if it makes sense to go even > > further and say there will only be async notifications for things like > > this. > > Yes, I thought the same initially. But then what about other existing MI > consumers? > > >From what I understood from Jonah's comments earlier, this would break > >(at least) > CDT. So CDT would either have to stick with MI2 (not great in a long term) > or refactor their code (not sure CDT guys would be happy to do so, > especially as - I presume - CDT needs to support wide range of GDB > versions already in the wild, a problem I do not have). > > While I personally agree with you and will be happy to go that far, it'd > break existing consumers - something that should IMO be carefully > discussed and planned. > > Adding a new option as I proposed as an alternative will be backward > compatible, indeed at the cost of more convoluted code in GDB itself. > > Is anyone from Emacs community around? Or any other MI consumers? Some "other (partial) MI consumer" here. And thank you for considering that possibility. 1. I also need to support a range of GDB versions (current lower limit is 7.4.1, but I guess I could bump that a bit if really needed) 2. Any kind of additional notification that does not change change sequencing or contents of messages of older versions sounds ok to me. 3. Output of gdb --version/show version, -list-features etc was in the past insufficient for me for feature discovery, so I typically use trial-and-error-and-fallback on some test input, or, in the areas where it is available, use the Python interface instead of MI. Insofar, I do not need backward compatibility in the sense that later versions need to continue to provide a specific feature, it's typically ok if a feature/command is either there or not. I believe completely removing features/command would be easier for me to handle than certain cases where there features only slightly change behaviour or need special setup to establish a "compatibility mode" or such. Andre' ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-10 21:19 MI3 and async notifications Jan Vrany 2019-06-10 23:23 ` Jonah Graham 2019-06-15 14:34 ` Tom Tromey @ 2019-06-18 3:14 ` Simon Marchi 2019-06-18 20:38 ` Jan Vrany 2 siblings, 1 reply; 26+ messages in thread From: Simon Marchi @ 2019-06-18 3:14 UTC (permalink / raw) To: Jan Vrany, gdb Cc: Joel Brobecker, Tom Tromey, André Pönitz, Jonah Graham On 2019-06-10 5:19 p.m., Jan Vrany wrote: > Hello, > > as an user og GDB/MI (and frontent developer), I'd like to > open a discussion about one aspect of MI that I'd like to change > in MI3 before it is released into the wild. Hi Jan and others, Thanks, nice to see some action on improving MI. I have just read the thread, and I have a few questions about where this is going. > Currently, quite a few commands suppress async notifications when > a command is issued through MI. For example, when a breakpoint is > added using -break-insert then =breakpoint-created is not emitted. > > At least in my case, this behavior complicates client design because > now the client has to care for new breakpoints on multiple places: > (i) listen to breakpoint created events to handle cases where breakpoint > is added via CLI. > (ii) do similar processing whenever MI returns ^done for previously > issued -break-insert command. > > This leads to a complex logic, especially in cases where frontend has > some scripting support (so execution of MI commands is not completely > under frontend developer's control). > > Emitting notifications unconditionally would simplify things a lot > - again at least in my case. The idea of the current behavior is that you shouldn't be asynchronously notified of the result of your own command. As was illustrated in this thread, it would be difficult to determine whether a =breakpoint-created async notification is the result of the pending -break-insert, or it just happens that the user has created a breakpoint on the CLI at the same time. So to remove this ambiguity, we don't send the notification. 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 problem, in which case please expand. 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. For example I would argue we are missing notifications if using two MI clients (through the new-ui command, although I don't remember if that's "officially" supported). If one of the MI client inserts a breakpoint with -break-insert, the other one is not notified. I would consider that a bug that the second client doesn't get a =breakpoint-created. 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. Simon ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-18 3:14 ` Simon Marchi @ 2019-06-18 20:38 ` Jan Vrany 2019-06-19 15:29 ` Simon Marchi 0 siblings, 1 reply; 26+ messages in thread From: Jan Vrany @ 2019-06-18 20:38 UTC (permalink / raw) To: Simon Marchi, gdb Cc: Joel Brobecker, Tom Tromey, André Pönitz, Jonah Graham 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? > > 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 :-) > For example I would argue we are missing notifications > if using two MI clients (through the new-ui command, although I don't remember if that's > "officially" supported). If one of the MI client inserts a breakpoint with -break-insert, > the other one is not notified. I would consider that a bug that the second client doesn't > get a =breakpoint-created. Yeah, having a second MI channel is a different story, I'm not considering this for now. > > 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. Thanks! Jan ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-18 20:38 ` Jan Vrany @ 2019-06-19 15:29 ` Simon Marchi 2019-06-19 20:58 ` Jan Vrany 0 siblings, 1 reply; 26+ messages in thread From: Simon Marchi @ 2019-06-19 15:29 UTC (permalink / raw) To: Jan Vrany, gdb Cc: Joel Brobecker, Tom Tromey, André Pönitz, Jonah Graham 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-19 15:29 ` Simon Marchi @ 2019-06-19 20:58 ` Jan Vrany 2019-06-20 15:31 ` Simon Marchi 0 siblings, 1 reply; 26+ messages in thread From: Jan Vrany @ 2019-06-19 20:58 UTC (permalink / raw) To: Simon Marchi, gdb Cc: Joel Brobecker, Tom Tromey, André Pönitz, Jonah Graham 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-19 20:58 ` Jan Vrany @ 2019-06-20 15:31 ` Simon Marchi 2019-06-20 20:46 ` Jan Vrany 2019-07-05 19:35 ` Pedro Alves 0 siblings, 2 replies; 26+ messages in thread From: Simon Marchi @ 2019-06-20 15:31 UTC (permalink / raw) To: Jan Vrany, gdb Cc: Joel Brobecker, Tom Tromey, André Pönitz, Jonah Graham On 2019-06-19 4:57 p.m., Jan Vrany wrote: > 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. For the -gdb-set case, this suggests that the ^done response should contain the new value. The reason being that, as you have shown, GDB can massage it a little bit. Since the value can end up different than what the frontend asked for, the frontend would need to be informed of the new/final value. >> 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 :-) Ok, then I understand how ugly it is to try to track the commands and their responses. I would tend to say that it falls in the category "you're not using it the way it was intended to": you basically have two or more interlocutors talking to GDB on the MI channel, whereas GDB expects to be talking to a single interlocutor. And your two or more interlocutors are not aware of each other's actions. It sounds like each "interlocutor" should have its own MI channel. Assuming the bug I mentioned earlier is fixed (about cross-MI channel notifications not being sent), the internals of your library (which maintains the model) would get notified whenever the user of your library creates a breakpoint with -break-insert. > 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). Ok, I had lost track of who suggested what in this long thread. I am still unsure about emitting events unconditionally. For example: some frontends want to insert "internal" breakpoints, which are breakpoints that are not going to be directly shown to users. They send a -break-insert and decide that the breakpoint which results from this won't be propagated to the UI. But breakpoint async events are propagated to the UI (presumably because they originate from the user creating a breakpoint in the CLI). If GDB now emits an event for the -break-insert breakpoint, the frontend can't know right away which future =breakpoint-created event it should ignore. Again, it would need to buffer all =breakpoint-created events until it gets the ^done, then let pass through all events except the one that matches the created breakpoint. Simon ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-20 15:31 ` Simon Marchi @ 2019-06-20 20:46 ` Jan Vrany 2019-07-05 19:35 ` Pedro Alves 1 sibling, 0 replies; 26+ messages in thread From: Jan Vrany @ 2019-06-20 20:46 UTC (permalink / raw) To: Simon Marchi, gdb Cc: Joel Brobecker, Tom Tromey, André Pönitz, Jonah Graham On Thu, 2019-06-20 at 11:31 -0400, Simon Marchi wrote: > > Ok, then I understand how ugly it is to try to track the commands and their responses. I would tend to > say that it falls in the category "you're not using it the way it was intended to": Fair enough :-) > > I am still unsure about emitting events unconditionally. For example: some frontends want to > insert "internal" breakpoints, which are breakpoints that are not going to be directly shown to > users. They send a -break-insert and decide that the breakpoint which results from this won't be > propagated to the UI. But breakpoint async events are propagated to the UI (presumably because > they originate from the user creating a breakpoint in the CLI). If GDB now emits an event for > the -break-insert breakpoint, the frontend can't know right away which future =breakpoint-created > event it should ignore. Again, it would need to buffer all =breakpoint-created events until it > gets the ^done, then let pass through all events except the one that matches the created breakpoint. > Yeah, that would be tricky. Okay, it looks I lost this case. That's fine. Thanks! Jan ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: MI3 and async notifications 2019-06-20 15:31 ` Simon Marchi 2019-06-20 20:46 ` Jan Vrany @ 2019-07-05 19:35 ` Pedro Alves 1 sibling, 0 replies; 26+ messages in thread From: Pedro Alves @ 2019-07-05 19:35 UTC (permalink / raw) To: Simon Marchi, Jan Vrany, gdb Cc: Joel Brobecker, Tom Tromey, André Pönitz, Jonah Graham On 6/20/19 4:31 PM, Simon Marchi wrote: > > I am still unsure about emitting events unconditionally. For example: some frontends want to > insert "internal" breakpoints, which are breakpoints that are not going to be directly shown to > users. They send a -break-insert and decide that the breakpoint which results from this won't be > propagated to the UI. But breakpoint async events are propagated to the UI (presumably because > they originate from the user creating a breakpoint in the CLI). If GDB now emits an event for > the -break-insert breakpoint, the frontend can't know right away which future =breakpoint-created > event it should ignore. Again, it would need to buffer all =breakpoint-created events until it > gets the ^done, then let pass through all events except the one that matches the created breakpoint. In the case of two MI channels, and assuming the issue with MI events not currently being broadcast to all channels/UIs is fixed, then such a -break-insert would show up as event in the other MI channel, and that other channel would have no way to know that that breakpoint is supposed to be internal. It would seem like this suggests that for such a use case, the breakpoint itself should carry some "this is an internal breakpoint" property. Note you can create internal breakpoints with Python today, but not with MI. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2019-07-05 21:58 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-10 21:19 MI3 and async notifications Jan Vrany 2019-06-10 23:23 ` Jonah Graham 2019-06-11 8:50 ` Jan Vrany 2019-06-11 13:37 ` Jonah Graham 2019-07-05 20:00 ` Pedro Alves 2019-07-05 21:58 ` Jonah Graham 2019-06-15 14:34 ` Tom Tromey 2019-06-17 10:53 ` Jan Vrany 2019-06-17 12:11 ` Jonah Graham 2019-06-17 12:14 ` Joel Brobecker 2019-06-17 12:26 ` Jonah Graham 2019-06-17 12:56 ` Joel Brobecker 2019-06-17 13:12 ` Jan Vrany 2019-06-17 13:23 ` Jonah Graham 2019-06-17 20:45 ` Joel Brobecker 2019-06-17 20:58 ` Jan Vrany 2019-06-17 21:50 ` Jonah Graham 2019-06-17 13:12 ` Jonah Graham 2019-06-17 19:52 ` André Pönitz 2019-06-18 3:14 ` Simon Marchi 2019-06-18 20:38 ` Jan Vrany 2019-06-19 15:29 ` Simon Marchi 2019-06-19 20:58 ` Jan Vrany 2019-06-20 15:31 ` Simon Marchi 2019-06-20 20:46 ` Jan Vrany 2019-07-05 19:35 ` Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).