Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
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 08:42:45 -0700	[thread overview]
Message-ID: <688f6c2f4f873e90d410c4e3b207cc20673aae1b.camel@us.ibm.com> (raw)
In-Reply-To: <875yzflgzs.fsf@linux.ibm.com>

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 


  reply	other threads:[~2021-05-20 15:43 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 [this message]
2021-05-20 16:01           ` Carl Love via Gdb-patches
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=688f6c2f4f873e90d410c4e3b207cc20673aae1b.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