Re: [PATCH] kill all threadapply processes at end of test

Carl Love via Gdb-patches
2021-05-20T16:01:49Z

Pedro, Simon, Tom:

Here is the updated patch to re-attach the threads and kill them. In
this version, I moved the re-attach and kill code to the
thr_apply_detach proceedure. This will kill off the detached threads
as soon as the test is done. The busy loop runs for a really long time
relative to the test.

The concern that was raised with the previous version of the patch was
the thread re-attach was done after all of the tests were completed.
The concern is that it is possible for a detatched thread to finish and
have the pid get reused before the code to re-attach and kill it
occurs. The result would be attaching to an unrelated pid and killing
it. By moving the re-attach code to the proceedure where the detach
testing is done reduces the time before killing the process. The
original issue was that the detatched processes were left running after
the entire testsuite finished. I saw these detached processes running
for several minutes after the gdb testsuite finished. Given the time
it takes for the busy loop to complete versus the time between the
thread being detached and reattached the likely hood of the process
finishing and the pid being recycled is extremely small.

The other proposal is to use thread barriors rather then the busy loop.
As pointed out, this could bread the other tests. The approach has not
been implemented so it is not clear if that would be a problem or not.
The barrior approach would guarentee that there is no possibility of a
pid being recycled, it does make the test code more complex.

Do people feel that the re-attach thread and kill it as implemented in
this patch is safe enough?

Carl

----------------------------------------------------

gdb/testsuite/ChangeLog:

2021-05-20 Carl Love <cel@us.ibm.com>

* gdb.threads/threadapply.exp (thr_apply_detach): Add test,
gdb_test_multiple, lappend pids to detach_pids.
Add foreach i loop on detach_pids, add gdb_test test to attach and
kill each pid.
---
gdb/testsuite/gdb.threads/threadapply.exp | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/gdb/testsuite/gdb.threads/threadapply.exp b/gdb/testsuite/gdb.threads/threadapply.exp
index ebc1cf15ad6..be34e38b50d 100644
--- a/gdb/testsuite/gdb.threads/threadapply.exp
+++ b/gdb/testsuite/gdb.threads/threadapply.exp
@@ -70,6 +70,8 @@ gdb_test "down" "#0.*thread_function.*" "go down and check selected frame"
# PR threads/13217.

proc thr_apply_detach {thread_set} {
+ set detach_pids ""
+
with_test_prefix "thread apply $thread_set" {
global binfile
global break_line
@@ -84,9 +86,29 @@ proc thr_apply_detach {thread_set} {
gdb_breakpoint "$break_line"
gdb_continue_to_breakpoint "all threads started"

+ set test "get inferior pid"
+ set pid -1
+
+ gdb_test_multiple "info inferior 1" "$test" {
+ -re "process (\[0-9\]*).*" {
+ set pid $expect_out(1,string)
+ pass "$test"
+ }
+ }
+
+ lappend detach_pids $pid
+
gdb_test "thread apply $thread_set detach" "Thread .*"
gdb_test "thread" "No thread selected" "switched to no thread selected"
}
+
+ # Don't leave detatched threadapply processes running. Reattach and stop
+ # the processes
+ foreach i $detach_pids {
+ # puts $i
+ gdb_test "attach $i" ".*" "Attach to $i" ".*A program is being debugged already.*" "y"
+ gdb_test "kill" ".*" "KILL $i" ".*Kill the program being debugged.*" "y"
+ }
}

# Test both "all" and a thread list, because those are implemented as
--
2.27.0