Re: [PATCH] PR threads/20743: Don't attempt to suspend or resume exited threads.

[Vasil Dimov <vd@FreeBSD.org>]

[-- Attachment #1: Type: text/plain, Size: 4236 bytes --]

On Fri, Dec 23, 2016 at 15:43:19 -0600, Luis Machado wrote:
> On 12/23/2016 03:28 PM, John Baldwin wrote:
> > When resuming a native FreeBSD process, ignore exited threads when
> > suspending/resuming individual threads prior to continuing the process.
> >
> > gdb/ChangeLog:
> >
> > 	PR threads/20743
> > 	* fbsd-nat.c (resume_one_thread_cb): Ignore exited threads.
> > 	(resume_all_threads_cb): Likewise.
> > 	(fbsd_resume): Assert resuming thread has not exited.
> > ---
> >  gdb/ChangeLog  | 7 +++++++
> >  gdb/fbsd-nat.c | 7 +++++++
> >  2 files changed, 14 insertions(+)
> >
> > diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> > index db6e913..4fb3732 100644
> > --- a/gdb/ChangeLog
> > +++ b/gdb/ChangeLog
> > @@ -1,3 +1,10 @@
> > +2016-12-23  John Baldwin  <jhb@FreeBSD.org>
> > +
> > +	PR threads/20743
> > +	* fbsd-nat.c (resume_one_thread_cb): Ignore exited threads.
> > +	(resume_all_threads_cb): Likewise.
> > +	(fbsd_resume): Assert resuming thread has not exited.
> > +
> >  2016-12-22  Doug Evans  <xdje42@gmail.com>
> >
> >  	* infrun.c (set_step_over_info): Add comment.
> > diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
> > index ade62f1..7cd08c6 100644
> > --- a/gdb/fbsd-nat.c
> > +++ b/gdb/fbsd-nat.c
> > @@ -662,6 +662,9 @@ resume_one_thread_cb (struct thread_info *tp, void *data)
> >    if (ptid_get_pid (tp->ptid) != ptid_get_pid (*ptid))
> >      return 0;
> >
> > +  if (is_exited (tp->ptid))
> > +    return 0;
> > +		
> >    if (ptid_get_lwp (tp->ptid) == ptid_get_lwp (*ptid))
> >      request = PT_RESUME;
> >    else
> > @@ -680,6 +683,9 @@ resume_all_threads_cb (struct thread_info *tp, void *data)
> >    if (!ptid_match (tp->ptid, *filter))
> >      return 0;
> >
> > +  if (is_exited (tp->ptid))
> > +    return 0;
> > +		
> >    if (ptrace (PT_RESUME, ptid_get_lwp (tp->ptid), NULL, 0) == -1)
> >      perror_with_name (("ptrace"));
> >    return 0;
> > @@ -711,6 +717,7 @@ fbsd_resume (struct target_ops *ops,
> >    if (ptid_lwp_p (ptid))
> >      {
> >        /* If ptid is a specific LWP, suspend all other LWPs in the process.  */
> > +      gdb_assert (!is_exited (ptid));
> 
> If we're asserting on this (since supposedly it shouldn't happen), do we 
> need to check for is_exited on the two functions above?
> 
> Also, is there a reason why we're not detecting a thread that has 
> exited? Aren't all threads stopped at this point (for all-stop mode at 
> least)?
[...]

Hello,

I just nailed this down after it has been annoying me for some time,
fixed it with a similar patch as the one submitted by John, and came
here to report it.

The reason that we are "not detecting" an exited thread (at least in the
scenario I got is), gdb/thread.c:

  --- cut ---
  static void
  delete_thread_1 (ptid_t ptid, int silent)
  {
  ...
    /* If this is the current thread, or there's code out there that
       relies on it existing (refcount > 0) we can't delete yet.  Mark
       it as exited, and notify it.  */
    if (tp->refcount > 0
        || ptid_equal (tp->ptid, inferior_ptid))
      {
  ...
         /* Will be really deleted some other time.  */
         printf_unfiltered ("========== Will be really deleted some other time %u\n", ptid);
         return;
       }
  ...
  if (tpprev)
    tpprev->next = tp->next;
  else
    thread_list = tp->next;
  --- cut ---

In my scenario tp->refcount is 0, but
"ptid_equal (tp->ptid, inferior_ptid)" is true, so the thread's entry is
not removed from the global "threads_list".

The gdb output (with "set debug fbsd-lwp" enabled):

  --- cut ---
  FLWP: adding thread for LWP 102009
  [New LWP 102009 of process 40304]
  FLWP: fbsd_resume for ptid (-1, 0, 0)
  FLWP: fbsd_resume for ptid (40304, 102009, 0)
  FLWP: fbsd_resume for ptid (-1, 0, 0)
  FLWP: fbsd_resume for ptid (40304, 102009, 0)
  FLWP: fbsd_resume for ptid (-1, 0, 0)
  FLWP: deleting thread for LWP 102009
  [LWP 102009 of process 40304 exited]
  ...
  ptrace: No such process.
  --- cut ---

Hope this helps.

-- 
Vasil Dimov
gro.DSBeerF@dv
%
There is no cure for birth and death other than to enjoy the interval.
                -- George Santayana

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]