[PATCH] kill all threadapply processes at end of test

[PATCH] kill all threadapply processes at end of test

[Carl Love]

GDB maintainers:

The threadapply test performs a number of tests to detatch and attach
threads.  When the test is all done, some detatched threads are left
running.  The following patch adds code to the end of the patch to kill
the processes that are still running.

Please let me know if this patch is acceptable.  Thanks.

                     Carl Love

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

gdb/testsuite/ChangeLog:

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

	* gdb.threads/threadapply.exp: Add foreach loop to kill all remaining
	threadapply processes.
---
 gdb/testsuite/gdb.threads/threadapply.exp | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gdb/testsuite/gdb.threads/threadapply.exp b/gdb/testsuite/gdb.threads/threadapply.exp
index ebc1cf15ad6..b3457121e76 100644
--- a/gdb/testsuite/gdb.threads/threadapply.exp
+++ b/gdb/testsuite/gdb.threads/threadapply.exp
@@ -229,3 +229,11 @@ proc kill_and_remove_inferior {thread_set} {
 foreach_with_prefix thread_set {"all" "1.1"} {
     kill_and_remove_inferior $thread_set
 }
+
+# Make sure all of the threadapply processes are terminated
+set data [exec ps -e | grep threadapply]
+
+foreach line [split $data \n] {
+    scan $line {%d %s} threadid junk
+    exec kill -9 $threadid
+}
-- 
2.27.0

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

[Simon Marchi]

Hi Carl,

On 2021-05-12 12:36 p.m., Carl Love via Gdb-patches wrote:
> The threadapply test performs a number of tests to detatch and attach
> threads.  When the test is all done, some detatched threads are left
> running.  The following patch adds code to the end of the patch to kill
> the processes that are still running.

From what I can see, there is not attach in that test, only detach.  GDB
starts some processes itself, and tests that detaching in a "thread
apply" command is correctly handled.  It's these processes that are
detached that are left running.  Fortunately, they exit when the
threads' counters go past INT_MAX, making the condition `*myp > 0`
false.  Unfortunately, it means they consume a lot of CPU for some time
after the test.

> Please let me know if this patch is acceptable.  Thanks.
> 
>                      Carl Love
> 
> --------------------------------------
> 
> gdb/testsuite/ChangeLog:
> 
> 2021-05-12  Carl Love  <cel@us.ibm.com>
> 
> 	* gdb.threads/threadapply.exp: Add foreach loop to kill all remaining
> 	threadapply processes.
> ---
>  gdb/testsuite/gdb.threads/threadapply.exp | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.threads/threadapply.exp b/gdb/testsuite/gdb.threads/threadapply.exp
> index ebc1cf15ad6..b3457121e76 100644
> --- a/gdb/testsuite/gdb.threads/threadapply.exp
> +++ b/gdb/testsuite/gdb.threads/threadapply.exp
> @@ -229,3 +229,11 @@ proc kill_and_remove_inferior {thread_set} {
>  foreach_with_prefix thread_set {"all" "1.1"} {
>      kill_and_remove_inferior $thread_set
>  }
> +
> +# Make sure all of the threadapply processes are terminated
> +set data [exec ps -e | grep threadapply]
> +
> +foreach line [split $data \n] {
> +    scan $line {%d %s} threadid junk
> +    exec kill -9 $threadid
> +}

We could use "kill" if we had no other choice, but it's a bit of a
rabbit hole to make that portable to all targets.  Instead, could we
redesign the test so there isn't this CPU-consuming loop?  Maybe use
a pthread barrier, such that when we detach, the all threads are
released and the program ends immediately.  I don't know if that will
cause complications because the threads' backtraces won't be exactly
predictable, but it would good to give this a try.

Simon

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

[Tom Tromey]

>>>>> "Carl" == Carl Love via Gdb-patches <gdb-patches@sourceware.org> writes:

Carl> +# Make sure all of the threadapply processes are terminated
Carl> +set data [exec ps -e | grep threadapply]

This seems like an issue in a few ways.  'ps' arguments may differ by
platform.  Other programs might be called 'threadapply' (unlikely for
user stuff but suppose I'm running 2 gdb test suites at the same time).
It doesn't handle cross-host testing (I don't know if we care about that
any more but in the past it was an issue).

So, I wonder if there's a better way to do this.
Like, could the test track PIDs itself?  Then maybe re-attach?

thanks,
Tom

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

[Carl Love]

Tom, Simon:

On Thu, 2021-05-13 at 10:30 -0400, Simon Marchi wrote:

> From what I can see, there is not attach in that test, only
> detach.  GDB
> starts some processes itself, and tests that detaching in a "thread
> apply" command is correctly handled.  It's these processes that are
> detached that are left running.  Fortunately, they exit when the
> threads' counters go past INT_MAX, making the condition `*myp > 0`
> false.  Unfortunately, it means they consume a lot of CPU for some
> time after the test.

Yes, that is a correct summary of the issue.

On Thu, 2021-05-13 at 09:44 -0600, Tom Tromey wrote:
> > > > > > "Carl" == Carl Love via Gdb-patches <
> > > > > > gdb-patches@sourceware.org> writes:
> 
> Carl> +# Make sure all of the threadapply processes are terminated
> Carl> +set data [exec ps -e | grep threadapply]
> 
> This seems like an issue in a few ways.  'ps' arguments may differ by
> platform.  Other programs might be called 'threadapply' (unlikely for
> user stuff but suppose I'm running 2 gdb test suites at the same
> time).
> It doesn't handle cross-host testing (I don't know if we care about
> that
> any more but in the past it was an issue).
> 
> So, I wonder if there's a better way to do this.
> Like, could the test track PIDs itself?  Then maybe re-attach?

I had tried to do that but couldn't get the code to work.

On Thu, 2021-05-13 at 10:30 -0400, Simon Marchi wrote:
> Instead, could we
> redesign the test so there isn't this CPU-consuming loop?  Maybe use
> a pthread barrier, such that when we detach, the all threads are
> released and the program ends immediately.  I don't know if that will
> cause complications because the threads' backtraces won't be exactly
> predictable, but it would good to give this a try.

Interesting idea.  I will give this a try.  If it doesn't work out, I
will go back and work on the re-attach approach some more to see if I
can get that to work.

Thanks for the help.

                    Carl 

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

[Carl Love]

Tom, Simon, GDB maintainers:

I looked at re-writing the test and using pthread barrier as suggested
by Simon.  I also went back and worked on my previous attempt to track
the pids to be detached, then at the end re-attach the pids.  I was
able to get the second approach to work.  The pthread barrier approach
didn't seem to be very obvious how to implement.

The following patch passes in a string to proc thr_apply_detach.  Each
pid is appended to the string before detaching from the pid.  After the
test is done, the test iterates thru the list of pids attaching to each
one then killing the process.  

The patch has been tested on Power to ensure that it passes
successfully and does not leave any threadapply processes running after
the test finishes. 

Please let me know if you have suggestions on the patch and if it is
accepatble to commit to mainline.  Thanks.

                             Carl 

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

gdb/testsuite/ChangeLog:

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

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

diff --git a/gdb/testsuite/gdb.threads/threadapply.exp b/gdb/testsuite/gdb.threads/threadapply.exp
index ebc1cf15ad6..d39a9798d7c 100644
--- a/gdb/testsuite/gdb.threads/threadapply.exp
+++ b/gdb/testsuite/gdb.threads/threadapply.exp
@@ -69,7 +69,7 @@ gdb_test "down" "#0.*thread_function.*" "go down and check selected frame"
 # exits due to the command run via thread apply.  Regression test for
 # PR threads/13217.
 
-proc thr_apply_detach {thread_set} {
+proc thr_apply_detach {thread_set detach_pids} {
     with_test_prefix "thread apply $thread_set" {
 	global binfile
 	global break_line
@@ -84,15 +84,30 @@ 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"
+	return $detach_pids
     }
 }
 
 # Test both "all" and a thread list, because those are implemented as
 # different commands in GDB.
+set detach_pids ""
+
 foreach thread_set {"all" "1.1 1.2 1.3"} {
-    thr_apply_detach $thread_set
+    set detach_pids [thr_apply_detach $thread_set $detach_pids]
 }
 
 # Test killing and removing inferiors from a command run via "thread
@@ -229,3 +244,10 @@ proc kill_and_remove_inferior {thread_set} {
 foreach_with_prefix thread_set {"all" "1.1"} {
     kill_and_remove_inferior $thread_set
 }
+
+# Make sure all of the threadapply processes are terminated
+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"
+}
-- 
2.27.0

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

[Pedro Franco de Carvalho]

Hello,

I didn't review the patch in detail, but one concern is that this could
cause a pid recycling issue. After expect gets the pid from the stopped
inferior, and the gdb under expect detaches from the inferior, it could
happen that the inferior finishes, there's lots and lots of processes in
the test system, and that same pid might get reused for a completely
different process, which GDB would attach to and kill, assuming it has
ptrace privileges over it. This might be unlikely, but it would be bad
for the testsuite to kill a random process in the system. I'm not sure
if there's an easy and portable solution to this.

Apart from that, it might be simpler to do the attaching and killing
inside proc thr_apply_detach.

Thanks,
Pedro Franco de Carvalho

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

[Pedro Franco de Carvalho]

Hi Carl,

Another thing, regarding pthread_barrier_wait solution that Simon
suggested.  I'm not sure, but I think the idea would be to initialize a
barrier with a count of as many threads as are created in the test, plus
the main thread.  Then you'd put a call to pthread_barrier_wait before
the exit from the thread_function, and remove the busy loop.  This will
be called by all the created threads shortly after they are created, but
they won't exit until the main itself thread also calls thread_function
and then pthread_barrier_wait.  Assuming the breakpoint in /* Break here
*/ for main happens before that, all threads will be alive for the test
when threadapply.exp calls 'thread apply detach', and they will only
exit after the detach, which would obviate the need for the busy loop.

This would however probably break the previous test with the backtraces,
like Simon said, because the backtrace could put the threads somewhere
in the call to ptrace_barrier_wait, and it might be deeper than one call
depending on the implementation.  If you can find a way to make it also
work with the barriers, and make sure that the other tests in
threadapply also work, that would allow removing the busy loops for all
test.

But if that isn't possible to do for the other tests, if you just want
to avoid having dangling threads, then one idea is to separate the
detach part of the test into another test entirely, that uses the
barrier and no busy loop, just for testing the detach case, or use
another .c for this part of the threadapply test. Not sure if it would
be acceptable to make a separate test.  But the barrier would avoid
having to deal with the pid recycling issue.

Thanks!
--
Pedro Franco de Carvalho

Pedro Franco de Carvalho via Gdb-patches <gdb-patches@sourceware.org>
writes:

> Hello,
>
> I didn't review the patch in detail, but one concern is that this could
> cause a pid recycling issue. After expect gets the pid from the stopped
> inferior, and the gdb under expect detaches from the inferior, it could
> happen that the inferior finishes, there's lots and lots of processes in
> the test system, and that same pid might get reused for a completely
> different process, which GDB would attach to and kill, assuming it has
> ptrace privileges over it. This might be unlikely, but it would be bad
> for the testsuite to kill a random process in the system. I'm not sure
> if there's an easy and portable solution to this.
>
> Apart from that, it might be simpler to do the attaching and killing
> inside proc thr_apply_detach.
>
> Thanks,
> Pedro Franco de Carvalho

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

[Carl Love]

Pedro:

On Tue, 2021-05-18 at 21:11 -0300, Pedro Franco de Carvalho wrote:
> Hi Carl,
> 
> Another thing, regarding pthread_barrier_wait solution that Simon
> suggested.  I'm not sure, but I think the idea would be to initialize
> a
> barrier with a count of as many threads as are created in the test,
> plus
> the main thread.  Then you'd put a call to pthread_barrier_wait
> before
> the exit from the thread_function, and remove the busy loop.  This
> will
> be called by all the created threads shortly after they are created,
> but
> they won't exit until the main itself thread also calls
> thread_function
> and then pthread_barrier_wait.  Assuming the breakpoint in /* Break
> here
> */ for main happens before that, all threads will be alive for the
> test
> when threadapply.exp calls 'thread apply detach', and they will only
> exit after the detach, which would obviate the need for the busy
> loop.
> 
> This would however probably break the previous test with the
> backtraces,
> like Simon said, because the backtrace could put the threads
> somewhere
> in the call to ptrace_barrier_wait, and it might be deeper than one
> call
> depending on the implementation.  If you can find a way to make it
> also
> work with the barriers, and make sure that the other tests in
> threadapply also work, that would allow removing the busy loops for
> all
> test.
> 
> But if that isn't possible to do for the other tests, if you just
> want
> to avoid having dangling threads, then one idea is to separate the
> detach part of the test into another test entirely, that uses the
> barrier and no busy loop, just for testing the detach case, or use
> another .c for this part of the threadapply test. Not sure if it
> would
> be acceptable to make a separate test.  But the barrier would avoid
> having to deal with the pid recycling issue.
> 

Moving the detach code to the end of thr_apply_detach simplifies the
patch significantly.  The current busy loop runs a really long time
relative to the length of the detatch tests.  So I do think that is a
very safe solution.

The ptrace_barrier_wait approach is definitely more complex and could
break the other tests as you described.  I have not implemented this
approach yet so I can say for sure what the issues may be.  The issue
is only with the detach tests so I can't see splitting the test into
two tests and then using the barrier method on the other tests that
don't have the issue of threads left running.  It is making those tests
more complex and not fixing an existing issue with those tests.  

I did move the re-attach thread code to the thr_apply_detach test and
it does seem to work. I will post it for review and discussion of the
pid recycling issue.

                          Carl 

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

[Carl Love]

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

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

[Carl Love]

Simon, Tom, Pedro, GCC maintainers:

I have implemented the test changes to use pthread barrier rather then
the spin loop.  The changes only require changing the test with no
changes to the expect scripts.  

The threadsapply test runs fine with no errors and no processes running
after the test with just the changes to threadapply.c.  This approach
seems to work well and would be better then the previous patches to
kill the processes in the expect script.  

The patch has been tested on Power.  Please let me know if you have any
additional comments or concern. Thanks.

                         Carl Love
----------------------------------------------------------------------
Change test to use barrier wait

gdb/testsuite/ChangeLog:

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

	* gdb.threads/threadapply.c: Add global mybarrier.
	(main): Add pthread_barrier_init.
	(thread_function): Replace while loop with myp increment and
	pthread_barrier_wait.
---
 gdb/testsuite/gdb.threads/threadapply.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.threads/threadapply.c b/gdb/testsuite/gdb.threads/threadapply.c
index 93a3fc8e82d..1ac99b07fc1 100644
--- a/gdb/testsuite/gdb.threads/threadapply.c
+++ b/gdb/testsuite/gdb.threads/threadapply.c
@@ -27,6 +27,7 @@ void *thread_function(void *arg); /* Pointer to function executed by each thread
 #define NUM 5
 
 unsigned int args[NUM+1];
+pthread_barrier_t mybarrier;
 
 int main() {
     int res;
@@ -35,6 +36,8 @@ int main() {
     void *thread_result;
     long i;
 
+    pthread_barrier_init(&mybarrier, NULL, NUM + 1);
+
     for (i = 0; i < NUM; i++)
       {
 	args[i] = 1; /* Init value.  */
@@ -69,12 +72,7 @@ void *thread_function(void *arg) {
     int my_number =  (long) arg;
     int *myp = (int *) &args[my_number];
 
-    /* Don't run forever.  Run just short of it :)  */
-    while (*myp > 0)
-      {
-	(*myp) ++;  /* Loop increment.  */
-      }
-
-    pthread_exit(NULL);
+    (*myp) ++;  /* Increment so parent knows child started.  */
+    pthread_barrier_wait(&mybarrier);
 }
 
-- 
2.27.0

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

[Simon Marchi]

On 2021-05-24 11:30 a.m., Carl Love wrote:
> Simon, Tom, Pedro, GCC maintainers:
> 
> I have implemented the test changes to use pthread barrier rather then
> the spin loop.  The changes only require changing the test with no
> changes to the expect scripts.  
> 
> The threadsapply test runs fine with no errors and no processes running
> after the test with just the changes to threadapply.c.  This approach
> seems to work well and would be better then the previous patches to
> kill the processes in the expect script.  
> 
> The patch has been tested on Power.  Please let me know if you have any
> additional comments or concern. Thanks.
> 
>                          Carl Love
> ----------------------------------------------------------------------
> Change test to use barrier wait
> 
> gdb/testsuite/ChangeLog:
> 
> 2021-05-22  Carl Love  <cel@us.ibm.com>
> 
> 	* gdb.threads/threadapply.c: Add global mybarrier.
> 	(main): Add pthread_barrier_init.
> 	(thread_function): Replace while loop with myp increment and
> 	pthread_barrier_wait.

Hi,

The code looks good to me, but please add a suitable commit message, it
should contain a description and explanation of the observed problem and
the chosen solution, such that somebody not familiar with the issue can
understand the rationale of the patch.

Simon

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

[Carl Love]

GDB maintainers, Simon:

On Fri, 2021-05-28 at 21:48 -0400, Simon Marchi wrote:
> <snip>
> Hi,
> 
> The code looks good to me, but please add a suitable commit message,
> it should contain a description and explanation of the observed
> problem and the chosen solution, such that somebody not familiar with
> the issue can understand the rationale of the patch.
> 
> Simon

I have added a commit message describing the issue with the test and
how the patch fixes the issue. 

Please let me know if the patch is acceptable for committing to GDB
mainline.   Thanks.

                   Carl

---------------------------------------------------------------------
Fix threadapply test

The current test case leaves detached processes running at the end of
the test.  This patch changes the test to use a barrier wait to ensure all
processes exit cleanly at the end of the tests.

gdb/testsuite/ChangeLog:

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

	* gdb.threads/threadapply.c: Add global mybarrier.
	(main): Add pthread_barrier_init.
	(thread_function): Replace while loop with myp increment and
	pthread_barrier_wait.
---
 gdb/testsuite/gdb.threads/threadapply.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.threads/threadapply.c b/gdb/testsuite/gdb.threads/threadapply.c
index 93a3fc8e82d..1ac99b07fc1 100644
--- a/gdb/testsuite/gdb.threads/threadapply.c
+++ b/gdb/testsuite/gdb.threads/threadapply.c
@@ -27,6 +27,7 @@ void *thread_function(void *arg); /* Pointer to function executed by each thread
 #define NUM 5
 
 unsigned int args[NUM+1];
+pthread_barrier_t mybarrier;
 
 int main() {
     int res;
@@ -35,6 +36,8 @@ int main() {
     void *thread_result;
     long i;
 
+    pthread_barrier_init(&mybarrier, NULL, NUM + 1);
+
     for (i = 0; i < NUM; i++)
       {
 	args[i] = 1; /* Init value.  */
@@ -69,12 +72,7 @@ void *thread_function(void *arg) {
     int my_number =  (long) arg;
     int *myp = (int *) &args[my_number];
 
-    /* Don't run forever.  Run just short of it :)  */
-    while (*myp > 0)
-      {
-	(*myp) ++;  /* Loop increment.  */
-      }
-
-    pthread_exit(NULL);
+    (*myp) ++;  /* Increment so parent knows child started.  */
+    pthread_barrier_wait(&mybarrier);
 }
 
-- 
2.27.0

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

[Simon Marchi]

> I have added a commit message describing the issue with the test and
> how the patch fixes the issue. 
> 
> Please let me know if the patch is acceptable for committing to GDB
> mainline.   Thanks.
> 
>                    Carl
> 
> ---------------------------------------------------------------------
> Fix threadapply test
> 
> The current test case leaves detached processes running at the end of
> the test.  This patch changes the test to use a barrier wait to ensure all
> processes exit cleanly at the end of the tests.
> 
> gdb/testsuite/ChangeLog:
> 
> 2021-05-22  Carl Love  <cel@us.ibm.com>
> 
> 	* gdb.threads/threadapply.c: Add global mybarrier.
> 	(main): Add pthread_barrier_init.
> 	(thread_function): Replace while loop with myp increment and
> 	pthread_barrier_wait.

Thanks, this is OK.  Do you have push access or know somebody that could
push it for you?  Otherwise, I can do it.

Simon

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

[Carl Love]

Simon:

On Tue, 2021-06-01 at 12:08 -0400, Simon Marchi wrote:
> Thanks, this is OK.  Do you have push access or know somebody that
> could
> push it for you?  Otherwise, I can do it.
> 

Now that it is approved, I can do the commit.  Thanks.

                         Carl