[PATCH 0/3] [gdb/testsuite] Clean up left-over processes

[PATCH 0/3] [gdb/testsuite] Clean up left-over processes

[Tom de Vries]

While investigating a testsuite problem using ps fx, I noticed a lot of
left-over processes.

I wrote a demonstrator patch to detect those in a less ad-hoc way, and
filed my findings as PR32709.

This series consists of three patches, and each patch fixes a different
root cause for this problem.

This fixes part of PR32709, I didn't manage the gdbserver part.

Tom de Vries (3):
  [gdb/testsuite] Don't start gdb in gdb.base/gstack.exp
  [gdb/testsuite] Kill left-over process in
    gdb.base/add-symbol-file-attach.exp
  [gdb/testsuite] Exit left-over gdb in gdb.mi/mi-break.exp

 .../gdb.base/add-symbol-file-attach.exp       |  3 ++
 gdb/testsuite/gdb.base/gstack.exp             |  4 +-
 gdb/testsuite/gdb.mi/mi-break.exp             |  4 ++
 gdb/testsuite/gdb.mi/mi-multi-commands.exp    |  4 ++
 gdb/testsuite/gdb.mi/mi-thread-bp-deleted.exp |  6 ++-
 .../gdb.mi/mi-thread-specific-bp.exp          | 41 +++++++++++++------
 gdb/testsuite/gdb.mi/mi-watch.exp             |  4 ++
 gdb/testsuite/gdb.mi/new-ui-bp-deleted.exp    |  6 ++-
 gdb/testsuite/gdb.mi/new-ui-mi-sync.exp       | 26 ++++++++----
 gdb/testsuite/gdb.mi/run-with-two-mi-uis.exp  |  4 ++
 .../gdb.mi/user-selected-context-sync.exp     | 35 ++++++++++------
 gdb/testsuite/lib/gdb.exp                     | 21 ++++++++++
 12 files changed, 118 insertions(+), 40 deletions(-)


base-commit: 114396befddc4104f9d7dd09b34e878bfaf440a8
-- 
2.43.0

[PATCH 1/3] [gdb/testsuite] Don't start gdb in gdb.base/gstack.exp

[Tom de Vries]

In test-case gdb.base/gstack.exp we start a gdb implicitly using
prepare_for_testing.

The gdb is not really used, but its spawn_id (available in variable
gdb_spawn_id) is used in a gdb_test_multiple, which is used to interact with
the gstack process.

Usually, a running gdb is cleaned up at test-case exit in gdb_finish, which
calls gdb_exit, which by default calls gdb_default_exit, which does
'send_gdb "quit\n"'.

However, this sends a quit to the host process expect is currently talking to,
defined by board_info(host,fileid), and after spawning gstack that's gstack, not
gdb.

Fix this by:
- using build_executable instead of prepare_for_testing to not spawn an unused
  gdb, and
- changing the gdb_test_multiple into a gdb_expect, eliminating the implicit use
  of gdb_spawn_id.

Tested on x86_64-linux.

PR testsuite/32709
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32709
---
 gdb/testsuite/gdb.base/gstack.exp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/gstack.exp b/gdb/testsuite/gdb.base/gstack.exp
index f603f2b4ea3..8df36b1b3bc 100644
--- a/gdb/testsuite/gdb.base/gstack.exp
+++ b/gdb/testsuite/gdb.base/gstack.exp
@@ -18,7 +18,7 @@ require can_spawn_for_attach
 
 standard_testfile
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug}] == -1} {
+if {[build_executable "failed to prepare" $testfile $srcfile {debug}] == -1} {
     return -1
 }
 
@@ -62,7 +62,7 @@ if { ![gdb_assert { ![expr {$res < 0 || $res == ""}] } $test] } {
 set test "got backtrace"
 set saw_backtrace false
 set no_awk false
-gdb_test_multiple "" $test {
+gdb_expect {
     -i "$res" -re "#0 +(0x\[0-9a-f\]+ in )?main \(\).*\r\nGSTACK-END\r\n\$" {
 	set saw_backtrace true
 	pass $test
-- 
2.43.0

Re: [PATCH 1/3] [gdb/testsuite] Don't start gdb in gdb.base/gstack.exp

[Keith Seitz]

Hi,

On 2/17/25 8:36 AM, Tom de Vries wrote:
> In test-case gdb.base/gstack.exp we start a gdb implicitly using
> prepare_for_testing.

Oh, ouch! That is totally my bad!

> Fix this by:
> - using build_executable instead of prepare_for_testing to not spawn an unused
>    gdb, and
> - changing the gdb_test_multiple into a gdb_expect, eliminating the implicit use
>    of gdb_spawn_id.

Thank you for catching this and cleaning it up. Truly appreciated.

Reviewed-By: Keith Seitz <keiths@redhat.com>

Keith

Re: [PATCH 1/3] [gdb/testsuite] Don't start gdb in gdb.base/gstack.exp

[Tom de Vries]

On 2/17/25 18:20, Keith Seitz wrote:
> Hi,
> 
> On 2/17/25 8:36 AM, Tom de Vries wrote:
>> In test-case gdb.base/gstack.exp we start a gdb implicitly using
>> prepare_for_testing.
> 
> Oh, ouch! That is totally my bad!
> 
>> Fix this by:
>> - using build_executable instead of prepare_for_testing to not spawn 
>> an unused
>>    gdb, and
>> - changing the gdb_test_multiple into a gdb_expect, eliminating the 
>> implicit use
>>    of gdb_spawn_id.
> 
> Thank you for catching this and cleaning it up. Truly appreciated.
> 

Hi Keith,

thanks for the review.

The problem really is that it's very easy to overlook this, because 
there's no explicit feedback about left-over processes.  Even more so if 
you run one test-case at a time, because the left-over processes are 
immediately cleaned up.

I'm hoping to eventually submit a patch that monitors and reports 
left-over processes.

For now, I've pushed this.

Thanks,
- Tom

> Reviewed-By: Keith Seitz <keiths@redhat.com>
> 
> Keith
> 

[PATCH 2/3] [gdb/testsuite] Kill left-over process in gdb.base/add-symbol-file-attach.exp

[Tom de Vries]

When running test-case gdb.base/add-symbol-file-attach.exp, the process
spawned by:
...
set test_spawn_id [spawn_wait_for_attach $binfile]
...
is not cleaned up.

The usual way to fix this is to add "kill_wait_spawned_process $test_spawn_id"
at the end of the test-case.

However, that doesn't look robust enough to me: in case of running into some
form of trouble in the test-case, that cleanup may not be reached.

I considered adding a wrapper proc like save_vars, but that makes fixing this
a bit intrusive since it requires re-indenting most of the test-case.

Instead, add a new proc at_finish_kill_wait:
...
 set test_spawn_id [spawn_wait_for_attach $binfile]
+at_finish_kill_wait $test_spawn_id
...
that schedules the cleanup for test-case finish.

While we're at it, make sure that we check for the result of
spawn_wait_for_attach, and since this:
...
UNSUPPORTED: $exp: require failed: !string equal $test_spawn_id "-1"
...
is somewhat hard to parse, add proc valid_spawn_id to get the more readable:
...
UNSUPPORTED: $exp: require failed: valid_spawn_id $test_spawn_id
...

Tested on x86_64-linux.

PR testsuite/32709
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32709
---
 .../gdb.base/add-symbol-file-attach.exp       |  3 +++
 gdb/testsuite/lib/gdb.exp                     | 21 +++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/gdb/testsuite/gdb.base/add-symbol-file-attach.exp b/gdb/testsuite/gdb.base/add-symbol-file-attach.exp
index e39a26ab666..97e0e018fc7 100644
--- a/gdb/testsuite/gdb.base/add-symbol-file-attach.exp
+++ b/gdb/testsuite/gdb.base/add-symbol-file-attach.exp
@@ -44,6 +44,9 @@ if {[build_executable $testfile.exp $testfile $srcfile debug] == -1} {
 # so that GDB may attach to it.
 
 set test_spawn_id [spawn_wait_for_attach $binfile]
+require {valid_spawn_id $test_spawn_id}
+at_finish_kill_wait $test_spawn_id
+
 set testpid [spawn_id_get_pid $test_spawn_id]
 
 gdb_start
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index d4634c85725..7c2d185b690 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -101,6 +101,16 @@ append_environment_default ASAN_OPTIONS detect_leaks 0
 # List of procs to run in gdb_finish.
 set gdb_finish_hooks [list]
 
+# List of spawn_ids to kill-and-wait-for-exit in gdb_finish.
+set gdb_finish_kill_wait {}
+
+# Schedule each arg for kill-and-wait-for-exit in gdb_finish.
+proc at_finish_kill_wait { args } {
+    foreach arg $args {
+	lappend ::gdb_finish_kill_wait $arg
+    }
+}
+
 # Variable in which we keep track of globals that are allowed to be live
 # across test-cases.
 array set gdb_persistent_globals {}
@@ -6913,6 +6923,12 @@ proc spawn_wait_for_attach { executable_list } {
     return [spawn_wait_for_attach_1 $executable_list]
 }
 
+# Return 1 if SPAWN_ID is a valid spawn id.
+
+proc valid_spawn_id { spawn_id } {
+    return [expr ![string equal $spawn_id "-1"]]
+}
+
 #
 # gdb_load_cmd -- load a file into the debugger.
 #		  ARGS - additional args to load command.
@@ -7884,6 +7900,11 @@ proc gdb_finish { } {
     set gdb_finish_hooks [list]
 
     gdb_cleanup_globals
+
+    foreach spawn_id $::gdb_finish_kill_wait {
+	kill_wait_spawned_process $spawn_id
+    }
+    set ::gdb_finish_kill_wait {}
 }
 
 global debug_format
-- 
2.43.0

Re: [PATCH 2/3] [gdb/testsuite] Kill left-over process in gdb.base/add-symbol-file-attach.exp

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> +require {valid_spawn_id $test_spawn_id}
Tom> +at_finish_kill_wait $test_spawn_id

Should this really yield unsupported, though, and not a fail?

Also I was wondering if at_finish_kill_wait should just be implicit in
the spawn.

thanks,
Tom

Re: [PATCH 2/3] [gdb/testsuite] Kill left-over process in gdb.base/add-symbol-file-attach.exp

[Tom de Vries]

On 2/24/25 18:26, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> +require {valid_spawn_id $test_spawn_id}
> Tom> +at_finish_kill_wait $test_spawn_id
> 
> Should this really yield unsupported, though, and not a fail?
> 

AFAIU, the only thing that FAIL should be used for, is if gdb is 
behaving incorrectly.  In this case, gdb is not involved, this is just 
expact spawning a process for gdb to attach to.

> Also I was wondering if at_finish_kill_wait should just be implicit in
> the spawn.

I thought about that as well, but I think there are cases were spawned 
processes are killed through other means.

Maybe we can do this by default, and then add a parameter to disable 
that to deal with those cases.

Thanks,
- Tom

Re: [PATCH 2/3] [gdb/testsuite] Kill left-over process in gdb.base/add-symbol-file-attach.exp

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

>> Should this really yield unsupported, though, and not a fail?

Tom> AFAIU, the only thing that FAIL should be used for, is if gdb is
Tom> behaving incorrectly.  In this case, gdb is not involved, this is just
Tom> expact spawning a process for gdb to attach to.

Yeah, that makes sense.  But I think it also makes sense to
differentiate between "cannot run because you don't have a C++ compiler"
and "spawn failed somehow but should not have".

The former is an expected thing -- you didn't want to test C++ or Ada or
whatever.

The latter is some kind of error.

Tom

Re: [PATCH 2/3] [gdb/testsuite] Kill left-over process in gdb.base/add-symbol-file-attach.exp

[Tom de Vries]

On 2/24/25 19:05, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
>>> Should this really yield unsupported, though, and not a fail?
> 
> Tom> AFAIU, the only thing that FAIL should be used for, is if gdb is
> Tom> behaving incorrectly.  In this case, gdb is not involved, this is just
> Tom> expact spawning a process for gdb to attach to.
> 
> Yeah, that makes sense.  But I think it also makes sense to
> differentiate between "cannot run because you don't have a C++ compiler"
> and "spawn failed somehow but should not have".
> 
> The former is an expected thing -- you didn't want to test C++ or Ada or
> whatever.
> 
> The latter is some kind of error.

True.

I don't think we have anything to express this currently.

One could consider ERROR, but IMO that's reserved for a problem in the 
test suite.  In this case, that may not necessarily be the case, the 
system may just have run out of enough memory to spawn a process.

I've suggested before to use UNSUPPORTED/UNTESTED for the difference 
between:
-  gdb doesn't supported it (say gdb is compiled with --disable-tui),
    and
- the environment doesn't support it,
and you could say that this is an instance of the latter.

In another case, I've suggested to use "UNSUPPORTED: <standard message>: 
$msg" for particular kinds of unsupported, like the case where a 
test-case needs specific support for an architecture, like some register 
or dwarf register number.

Thanks,
- Tom

[PATCH 3/3] [gdb/testsuite] Exit left-over gdb in gdb.mi/mi-break.exp

[Tom de Vries]

After test-case gdb.mi/mi-break.exp, a gdb instance is left running.

The test-case starts two instances using mi_clean_restart, one using
separate-mi-tty.

For each instance, gdb_exit is called once, from two different locations:
- mi_clean_restart, and
- gdb_finish.

But this doesn't seem to be effective for the separate-mi-tty case.

Fix this by calling gdb_mi_exit at the end of proc test_break.

Likewise in a few more more test-case.

Tested on x86_64-linux.

PR testsuite/32709
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32709
---
 gdb/testsuite/gdb.mi/mi-break.exp             |  4 ++
 gdb/testsuite/gdb.mi/mi-multi-commands.exp    |  4 ++
 gdb/testsuite/gdb.mi/mi-thread-bp-deleted.exp |  6 ++-
 .../gdb.mi/mi-thread-specific-bp.exp          | 41 +++++++++++++------
 gdb/testsuite/gdb.mi/mi-watch.exp             |  4 ++
 gdb/testsuite/gdb.mi/new-ui-bp-deleted.exp    |  6 ++-
 gdb/testsuite/gdb.mi/new-ui-mi-sync.exp       | 26 ++++++++----
 gdb/testsuite/gdb.mi/run-with-two-mi-uis.exp  |  4 ++
 .../gdb.mi/user-selected-context-sync.exp     | 35 ++++++++++------
 9 files changed, 92 insertions(+), 38 deletions(-)

diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
index 86e7b57260a..aa8603f4ef0 100644
--- a/gdb/testsuite/gdb.mi/mi-break.exp
+++ b/gdb/testsuite/gdb.mi/mi-break.exp
@@ -417,6 +417,10 @@ proc test_break {mi_mode} {
     test_explicit_breakpoints
 
     test_forced_conditions
+
+    # mi_clean_restart and gdb_finish call gdb_exit, which doesn't work for
+    # separate-mi-tty.  Use mi_gdb_exit instead.
+    mi_gdb_exit
 }
 
 foreach_mi_ui_mode mi-mode {
diff --git a/gdb/testsuite/gdb.mi/mi-multi-commands.exp b/gdb/testsuite/gdb.mi/mi-multi-commands.exp
index 028e187366a..ce4379c8636 100644
--- a/gdb/testsuite/gdb.mi/mi-multi-commands.exp
+++ b/gdb/testsuite/gdb.mi/mi-multi-commands.exp
@@ -123,6 +123,10 @@ proc run_test { args } {
 	    break
 	}
     }
+
+    # mi_clean_restart and gdb_finish call gdb_exit, which doesn't work for
+    # separate-mi-tty.  Use mi_gdb_exit instead.
+    mi_gdb_exit
 }
 
 foreach_with_prefix args { "" "separate-mi-tty" } {
diff --git a/gdb/testsuite/gdb.mi/mi-thread-bp-deleted.exp b/gdb/testsuite/gdb.mi/mi-thread-bp-deleted.exp
index 08c72182217..261ab7d0369 100644
--- a/gdb/testsuite/gdb.mi/mi-thread-bp-deleted.exp
+++ b/gdb/testsuite/gdb.mi/mi-thread-bp-deleted.exp
@@ -36,8 +36,6 @@ if { [build_executable "failed to prepare" $testfile $srcfile \
 }
 
 foreach_mi_ui_mode mode {
-    mi_gdb_exit
-
     if {$mode eq "separate"} {
 	set start_ops "separate-mi-tty"
     } else {
@@ -279,4 +277,8 @@ foreach_mi_ui_mode mode {
 	"set do_spin variable in inferior, inferior should now finish"
     mi_expect_stop "breakpoint-hit" "breakpt" ".*" ".*" "$::decimal" \
 	{"" "disp=\"keep\""} "stop in breakpt at the end of the test"
+
+    # mi_clean_restart and gdb_finish call gdb_exit, which doesn't work for
+    # separate-mi-tty.  Use mi_gdb_exit instead.
+    mi_gdb_exit
 }
diff --git a/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
index 715e0f7e804..6026e7b4727 100644
--- a/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
+++ b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
@@ -33,17 +33,7 @@ proc make_loc {num} {
     return [mi_make_breakpoint_loc -thread "1" -number "$::decimal\\.$num"]
 }
 
-foreach_mi_ui_mode mode {
-
-    if {$mode == "separate"} {
-	set start_ops "separate-mi-tty"
-    } else {
-	set start_ops ""
-    }
-
-    if {[mi_clean_restart $binfile $start_ops]} {
-	return -1
-    }
+proc do_test { mode } {
 
     # Ensure we get an error when placing a b/p for thread 1 at a point
     # where thread 1 doesn't exist.
@@ -55,7 +45,7 @@ foreach_mi_ui_mode mode {
     # would succeed, and then 'info breakpoint' on the CLI would
     # trigger an assertion.
     if {$mode eq "separate"} {
-	with_spawn_id $gdb_main_spawn_id {
+	with_spawn_id $::gdb_main_spawn_id {
 	    gdb_test "info breakpoints" "No breakpoints, watchpoints, tracepoints, or catchpoints\\." \
 		"check CLI 'info breakpoints' when there are no breakpoints"
 	}
@@ -83,9 +73,34 @@ foreach_mi_ui_mode mode {
 
     # Check that 'info breakpoints' on the CLI succeeds.
     if {$mode eq "separate"} {
-	with_spawn_id $gdb_main_spawn_id {
+	with_spawn_id $::gdb_main_spawn_id {
 	    gdb_test "info breakpoints" ".*" \
 		"check CLI 'info breakpoints' when there are some breakpoints"
 	}
     }
+
+    return 1
+}
+
+foreach_mi_ui_mode mode {
+
+    if {$mode == "separate"} {
+	set start_ops "separate-mi-tty"
+    } else {
+	set start_ops ""
+    }
+
+    if {[mi_clean_restart $binfile $start_ops]} {
+	break
+    }
+
+    set res [do_test $mode]
+
+    # mi_clean_restart and gdb_finish call gdb_exit, which doesn't work for
+    # separate-mi-tty.  Use mi_gdb_exit instead.
+    mi_gdb_exit
+
+    if { $res == -1 } {
+	break
+    }
 }
diff --git a/gdb/testsuite/gdb.mi/mi-watch.exp b/gdb/testsuite/gdb.mi/mi-watch.exp
index a5a442eb13e..cf97d416535 100644
--- a/gdb/testsuite/gdb.mi/mi-watch.exp
+++ b/gdb/testsuite/gdb.mi/mi-watch.exp
@@ -173,6 +173,10 @@ proc test_watchpoint_all {mi_mode type} {
     #test_rwatch_creation_and_listing
     #test_awatch_creation_and_listing
     test_watchpoint_triggering
+
+    # mi_clean_restart and gdb_finish call gdb_exit, which doesn't work for
+    # separate-mi-tty.  Use mi_gdb_exit instead.
+    mi_gdb_exit
 }
 
 # Run the tests twice, once using software watchpoints, and another
diff --git a/gdb/testsuite/gdb.mi/new-ui-bp-deleted.exp b/gdb/testsuite/gdb.mi/new-ui-bp-deleted.exp
index 938e6deec05..9e529880814 100644
--- a/gdb/testsuite/gdb.mi/new-ui-bp-deleted.exp
+++ b/gdb/testsuite/gdb.mi/new-ui-bp-deleted.exp
@@ -45,8 +45,6 @@ proc make_bp_loc { num } {
 }
 
 foreach_mi_ui_mode mode {
-    mi_gdb_exit
-
     if {$mode eq "separate"} {
 	set start_ops "separate-mi-tty"
     } else {
@@ -109,4 +107,8 @@ foreach_mi_ui_mode mode {
 		 "=breakpoint-deleted,id=\"$bpnum\"" \
 		 "\\^done"]
     }
+
+    # mi_clean_restart and gdb_finish call gdb_exit, which doesn't work for
+    # separate-mi-tty.  Use mi_gdb_exit instead.
+    mi_gdb_exit
 }
diff --git a/gdb/testsuite/gdb.mi/new-ui-mi-sync.exp b/gdb/testsuite/gdb.mi/new-ui-mi-sync.exp
index 28a42dcb665..bf1365e18ad 100644
--- a/gdb/testsuite/gdb.mi/new-ui-mi-sync.exp
+++ b/gdb/testsuite/gdb.mi/new-ui-mi-sync.exp
@@ -40,27 +40,22 @@ proc do_test {sync_command} {
     global gdb_spawn_id gdb_main_spawn_id mi_spawn_id inferior_spawn_id
     global gdb_prompt mi_gdb_prompt
 
-    if {[mi_clean_restart $binfile "separate-mi-tty"] != 0} {
-	fail "could not start gdb"
-	return
-    }
-
     # Start a synchronous run/continue on the MI UI.
     set test "send synchronous execution command"
     if {$sync_command == "run"} {
 	if {[mi_run_cmd] >= 0} {
 	    pass $test
 	} else {
-	    return
+	    return -1
 	}
     } else {
 	if {[mi_runto_main] < 0} {
-	    return
+	    return -1
 	}
 	if {[mi_send_resuming_command_raw "123-exec-continue" $test] >= 0} {
 	    pass $test
 	} else {
-	    return
+	    return -1
 	}
     }
 
@@ -107,5 +102,18 @@ proc do_test {sync_command} {
 }
 
 foreach_with_prefix sync-command {"run" "continue"} {
-    do_test ${sync-command}
+    if {[mi_clean_restart $binfile "separate-mi-tty"] != 0} {
+	fail "could not start gdb"
+	break
+    }
+
+    set res [do_test ${sync-command}]
+
+    # mi_clean_restart and gdb_finish call gdb_exit, which doesn't work for
+    # separate-mi-tty.  Use mi_gdb_exit instead.
+    mi_gdb_exit
+
+    if { $res == -1 } {
+	break
+    }
 }
diff --git a/gdb/testsuite/gdb.mi/run-with-two-mi-uis.exp b/gdb/testsuite/gdb.mi/run-with-two-mi-uis.exp
index 5981bc1ad24..effc760d6a7 100644
--- a/gdb/testsuite/gdb.mi/run-with-two-mi-uis.exp
+++ b/gdb/testsuite/gdb.mi/run-with-two-mi-uis.exp
@@ -59,6 +59,10 @@ proc do_test { ui_to_run } {
 	# the expected ^running record.
 	mi_runto_main
     }
+
+    # mi_clean_restart and gdb_finish call gdb_exit, which doesn't work for
+    # separate-mi-tty.  Use mi_gdb_exit instead.
+    mi_gdb_exit
 }
 
 foreach_with_prefix ui_to_run {first second} {
diff --git a/gdb/testsuite/gdb.mi/user-selected-context-sync.exp b/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
index e168a5eee45..675965fec60 100644
--- a/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
+++ b/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
@@ -385,21 +385,10 @@ proc_with_prefix test_setup { mode } {
     global mi_spawn_id
     global decimal
     global binfile
-    global GDBFLAGS
     global async
 
     set any "\[^\r\n\]*"
 
-    save_vars { GDBFLAGS } {
-	if { $mode == "non-stop" } {
-	    set GDBFLAGS [concat $GDBFLAGS " -ex \"set non-stop 1\""]
-	}
-
-	if { [mi_clean_restart $binfile "separate-mi-tty"] != 0 } {
-	    return -1
-	}
-    }
-
     if { [mi_runto_main] < 0 } {
 	return -1
     }
@@ -1300,7 +1289,7 @@ proc_with_prefix test_cli_in_mi_frame { mode cli_in_mi_mode } {
     }
 }
 
-foreach_with_prefix mode { "all-stop" "non-stop" } {
+proc do_test { mode } {
     set test "setup done"
     if { [test_setup $mode] == -1 } {
 	fail $test
@@ -1331,3 +1320,25 @@ foreach_with_prefix mode { "all-stop" "non-stop" } {
 	test_cli_in_mi_frame $mode $exec_mode
     }
 }
+
+foreach_with_prefix mode { "all-stop" "non-stop" } {
+    save_vars { GDBFLAGS } {
+	if { $mode == "non-stop" } {
+	    set GDBFLAGS [concat $GDBFLAGS " -ex \"set non-stop 1\""]
+	}
+
+	if { [mi_clean_restart $binfile "separate-mi-tty"] != 0 } {
+	    break
+	}
+    }
+
+    set res [do_test $mode]
+
+    # mi_clean_restart and gdb_finish call gdb_exit, which doesn't work for
+    # separate-mi-tty.  Use mi_gdb_exit instead.
+    mi_gdb_exit
+
+    if { $res == -1 } {
+	break
+    }
+}
-- 
2.43.0

Re: [PATCH 3/3] [gdb/testsuite] Exit left-over gdb in gdb.mi/mi-break.exp

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Fix this by calling gdb_mi_exit at the end of proc test_break.

Tom> Likewise in a few more more test-case.

Seems reasonable to me.
Approved-By: Tom Tromey <tom@tromey.com>

Tom