[binutils-gdb] Do not send queries on secondary UIs

[Simon Marchi <simark at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=26a06916b684ceda25c2edb43141e1101bf337c6

commit 26a06916b684ceda25c2edb43141e1101bf337c6
Author: Simon Marchi <simon.marchi@ericsson.com>
Date:   Fri Feb 10 16:29:50 2017 -0500

    Do not send queries on secondary UIs
    
    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.

Diff:
---
 gdb/ChangeLog                     |  4 ++++
 gdb/testsuite/ChangeLog           |  5 +++++
 gdb/testsuite/gdb.base/new-ui.exp | 12 ++++++++++++
 gdb/utils.c                       |  4 +++-
 4 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index dbee02a..dcd047a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2017-02-10  Simon Marchi  <simon.marchi@ericsson.com>
+
+	* utils.c (defaulted_query): Don't query on secondary UIs.
+
 2017-02-10  Tom Tromey  <tom@tromey.com>
 
 	* rust-lang.c (rust_get_disr_info): Remove unused variable.
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 3274b40..c1e53b7 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2017-02-10  Simon Marchi  <simon.marchi@ericsson.com>
 
+	* gdb.base/new-ui.exp (do_test): Test queries behavior on main
+	and extra UIs.
+
+2017-02-10  Simon Marchi  <simon.marchi@ericsson.com>
+
 	* gdb.base/new-ui.exp (do_test, do_test_invalid_args): Use
 	proc_with_prefix.
 
diff --git a/gdb/testsuite/gdb.base/new-ui.exp b/gdb/testsuite/gdb.base/new-ui.exp
index 8224b7d..b84f4de 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 3dc2f03..27021a1 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 ();