From: Carl Love via Gdb-patches <gdb-patches@sourceware.org>
To: Pedro Franco de Carvalho <pedromfc@linux.ibm.com>,
gdb-patches@sourceware.org
Cc: rogealve@br.ibm.com, ulrich.weigand@de.ibm.com, tom@tromey.com,
cel@us.ibm.com
Subject: Re: [PATCH] kill all threadapply processes at end of test
Date: Thu, 20 May 2021 09:01:29 -0700 [thread overview]
Message-ID: <73bcfb777e72f549218cb431784e103b1831ea30.camel@us.ibm.com> (raw)
In-Reply-To: <875yzflgzs.fsf@linux.ibm.com>
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
next prev parent reply other threads:[~2021-05-20 16:01 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-12 16:36 Carl Love via Gdb-patches
2021-05-13 14:30 ` Simon Marchi via Gdb-patches
2021-05-13 15:44 ` Tom Tromey
2021-05-13 18:10 ` Carl Love via Gdb-patches
2021-05-18 15:29 ` Carl Love via Gdb-patches
2021-05-18 21:56 ` Pedro Franco de Carvalho via Gdb-patches
2021-05-19 0:11 ` Pedro Franco de Carvalho via Gdb-patches
2021-05-20 15:42 ` Carl Love via Gdb-patches
2021-05-20 16:01 ` Carl Love via Gdb-patches [this message]
2021-05-24 15:30 ` Carl Love via Gdb-patches
2021-05-29 1:48 ` Simon Marchi via Gdb-patches
2021-06-01 15:42 ` Carl Love via Gdb-patches
2021-06-01 16:08 ` Simon Marchi via Gdb-patches
2021-06-02 15:08 ` Carl Love via Gdb-patches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=73bcfb777e72f549218cb431784e103b1831ea30.camel@us.ibm.com \
--to=gdb-patches@sourceware.org \
--cc=cel@us.ibm.com \
--cc=pedromfc@linux.ibm.com \
--cc=rogealve@br.ibm.com \
--cc=tom@tromey.com \
--cc=ulrich.weigand@de.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox