This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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 ();
>  
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]