This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/2] Do not send queries on secondary UIs
OK.
Thanks,
Pedro Alves
On 02/10/2017 09:02 PM, Simon Marchi wrote:
> This is a follow-up to
>
> https://sourceware.org/ml/gdb-patches/2017-02/msg00261.html
>
> This patch restricts queries to the main UI, which allows to avoid two
> different problems.
>
> The first one is that GDB is issuing queries on secondary MI channels
> for which a TTY is allocated. The second one is that GDB is not able to
> handle queries on two (CLI) UIs simultaneously. Restricting queries to
> the main UI allows to bypass these two problems.
>
> More details on how/why these two problems happen:
>
> 1. Queries on secondary MI UI
>
> The current criterion to decide if we should query the user is whether
> the input stream is a TTY. The original way to start GDB in MI mode
> from a front-end was to create a subprocess with pipes to its
> stdin/stdout. In this case, the input was considered non-interactive
> and queries were auto-answered. Now that front-ends can create the MI
> channel as a separate UI connected to a dedicated TTY, GDB now
> considers this input stream as interactive and sends queries to it.
> By restricting queries to the main UI, we make sure we never query on
> the secondary MI UI.
>
> 2. Simultaneous queries
>
> As Pedro stated it, when you have two queries on two different CLI UIs
> at the same time, you end up with the following pseudo stack:
>
> #0 gdb_readline_wrapper
> #1 defaulted_query // for UI #2
> #2 handle_command
> #3 execute_command ("handle SIGTRAP" ....
> #4 stdin_event_handler // input on UI #2
> #5 gdb_do_one_event
> #7 gdb_readline_wrapper
> #8 defaulted_query // for UI #1
> #9 handle_command
> #10 execute_command ("handle SIGINT" ....
> #11 stdin_event_handler // input on UI #1
> #12 gdb_do_one_event
> #13 gdb_readline_wrapper
>
> trying to answer the query on UI #1 will therefore answer for UI #2.
>
> By restricting the queries to the main UI, we ensure that there will
> never be more than one pending query, since you can't have two queries
> on a UI at the same time.
>
> I added a snippet to gdb.base/new-ui.exp to verify that we get a query
> on the main UI, but that we don't on the secondary one (or, more
> precisely, that it gets auto-answered).
>
> gdb/ChangeLog:
>
> * utils.c (defaulted_query): Don't query on secondary UIs.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/new-ui.exp (do_test): Test queries behavior on main
> and extra UIs.
> ---
> gdb/testsuite/gdb.base/new-ui.exp | 12 ++++++++++++
> gdb/utils.c | 4 +++-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/testsuite/gdb.base/new-ui.exp b/gdb/testsuite/gdb.base/new-ui.exp
> index 8224b7d615..b84f4de712 100644
> --- a/gdb/testsuite/gdb.base/new-ui.exp
> +++ b/gdb/testsuite/gdb.base/new-ui.exp
> @@ -128,6 +128,18 @@ proc_with_prefix do_test {} {
> gdb_test "print 2" "^print 2\r\n\\\$2 = 2" "print on extra console"
> }
>
> + # Verify that we get proper queries on the main UI, but that they are
> + # auto-answered on secondary UIs.
> + with_spawn_id $gdb_main_spawn_id {
> + gdb_test "delete" "" "delete all breakpoint on main console" \
> + "Delete all breakpoints. .y or n. $" "n"
> + }
> + with_spawn_id $extra_spawn_id {
> + gdb_test "delete" \
> + "Delete all breakpoints. .y or n. .answered Y; input not from terminal." \
> + "delete all breakpoints on extra console"
> + }
> +
> # Run a few execution tests with the main console as the driver
> # console.
> with_test_prefix "main console" {
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 3dc2f03d61..27021a1d45 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -1175,7 +1175,9 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
> way, important error messages don't get lost when talking to GDB
> over a pipe. */
> if (current_ui->instream != current_ui->stdin_stream
> - || !input_interactive_p (current_ui))
> + || !input_interactive_p (current_ui)
> + /* Restrict queries to the main UI. */
> + || current_ui != main_ui)
> {
> old_chain = make_cleanup_restore_target_terminal ();
>
>