For patch submissions and discussion

Daniel Jacobowitz | 1 Jul 2008 16:03

Re: [non-stop] 10/10 split user/internal threads

On Tue, Jul 01, 2008 at 02:37:44PM +0100, Pedro Alves wrote:
> A Tuesday 01 July 2008 01:51:28, Daniel Jacobowitz wrote:
> > On Mon, Jun 30, 2008 at 01:08:48AM +0100, Pedro Alves wrote:
> > > - In non-stop mode, the current thread may exit while the
> > >   user has it selected.  Switching current thread, and restoring
> > >   it with a cleanup is problematic in non-stop mode.
> >
> > The target interface is async, the inferior program is running - but
> > GDB retains a single thread of control.  So unless the inferior runs
> > while the cleanup is live, there's no problem here.  I suppose it
> > could be trouble if you enter the expression evaluator, though?
> 
> Sure it can run while the cleanup is live.  It could already be
> running when the cleanup was created.  Remember that the selected
> thread may be running at any time.

Doesn't matter if the thread is already running.  We won't go looking
for events while normal cleanups are on the stack in most cases - so
even if the thread exits, we won't remove it from the thread list
until later.  But the evaluator, and some commands, might cause us to
return to w_f_i.  That's all I meant.

> The user loses the selected frame in thread 1, because when
> the cleanup is ran, the thread is running/stepping.
> Since the thread stepped into another frame, we could
> claim that since the originally selected frame is still live,
> it should still be selected.  But, it can also be
> claimed that, though luck, the thread was set running,
> you lose the selected frame.  This doesn't seem
> like an important enough case to care about.  Agree?

(Continue reading)

headers

Pedro Alves | 2 Jul 2008 05:39

[non-stop] 10/10 handle exited threads [was: Re: [non-stop] 10/10 split user/internal threads]

Here's the rewrite of this patch to use cleanups and refcounting on the
thread_info objects to prevent them from being deleted while the thread
is held in a cleanup.  This version completelly drops the
external/internal thread separation, and sticks with inferior_ptid
to mean current thread.  Exited threads are kept in the thread list
until safe to delete.  Most commands aren't allowed while the
user has an exited thread selected.

Tested on x86_64-unknown-linux-gnu -m32, sync/async.

--

-- 
Pedro Alves

Attachment (010-exited_threads_ref_count.diff): text/x-diff, 44 KiB

headers

Daniel Jacobowitz | 7 Jul 2008 20:59

Re: [non-stop] 10/10 handle exited threads [was: Re: [non-stop] 10/10 split user/internal threads]

On Wed, Jul 02, 2008 at 04:39:07AM +0100, Pedro Alves wrote:
> 	(print_thread_info): Update.  Account for exited threads.  Don't
> 	warn about missed frame restoring here, it's done in the cleanup.

its

> +	  /* Since the context is already set to this new thread,
> +	     reset it's ptid, and reswitch inferior_ptid to it.  */

and here, too.

>  <at>  <at>  -163,6 +223,10  <at>  <at>  add_thread (ptid_t ptid)
>    return add_thread_with_info (ptid, NULL);
>  }
>  
> +/* Delete thread PTID and notify of thread exit.  If this is
> +   inferior_ptid, don't actually delete it, but tag it as exited and
> +   do the notification.  If PTID is the user selected thread, clear
> +   it.  */
>  void
>  delete_thread (ptid_t ptid)
>  {
>  <at>  <at>  -177,12 +241,35  <at>  <at>  delete_thread (ptid_t ptid)
>    if (!tp)
>      return;
>  
> +  /* If this is the current thread, or there's code out there that
> +     relies on it existing (refcout > 0) we can't delete yet.  Mark it
> +     as exited, and notify it.  */

(Continue reading)

headers

Pedro Alves | 11 Jul 2008 13:16

Re: [non-stop] 10/10 handle exited threads [was: Re: [non-stop] 10/10 split user/internal threads]

A Monday 07 July 2008 19:59:30, Daniel Jacobowitz wrote:

> On Wed, Jul 02, 2008 at 04:39:07AM +0100, Pedro Alves wrote:
> > 	(print_thread_info): Update.  Account for exited threads.  Don't
> > 	warn about missed frame restoring here, it's done in the cleanup.
>
> its

Fixed.

>
> > +	  /* Since the context is already set to this new thread,
> > +	     reset it's ptid, and reswitch inferior_ptid to it.  */
>
> and here, too.

Fixed.

>
> >  <at>  <at>  -163,6 +223,10  <at>  <at>  add_thread (ptid_t ptid)
> >    return add_thread_with_info (ptid, NULL);
> >  }
> >
> > +/* Delete thread PTID and notify of thread exit.  If this is
> > +   inferior_ptid, don't actually delete it, but tag it as exited and
> > +   do the notification.  If PTID is the user selected thread, clear
> > +   it.  */
> >  void
> >  delete_thread (ptid_t ptid)
> >  {

(Continue reading)

headers

Pedro Alves | 11 Jul 2008 13:28

Re: [non-stop] 10/10 handle exited threads [was: Re: [non-stop] 10/10 split user/internal threads]

On Friday 11 July 2008 12:16:15, Pedro Alves wrote:
> On Monday 07 July 2008 19:59:30, Daniel Jacobowitz wrote:

> > >  <at>  <at>  -877,18 +1057,10  <at>  <at>  thread_apply_command (char *tidlist, int
> > >    if (*cmd == '\000')
> > >      error (_("Please specify a command following the thread ID
> > > list"));
> > >
> > > -  current_ptid = inferior_ptid;
> > > -
> > > -  if (!is_running (inferior_ptid))
> > > -    saved_frame_id = get_frame_id (get_selected_frame (NULL));
> > > -  else
> > > -    saved_frame_id = null_frame_id;
> > > -  old_chain = make_cleanup_restore_current_thread (inferior_ptid,
> > > saved_frame_id); -
> > >    /* Save a copy of the command in case it is clobbered by
> > >       execute_command */
> > >    saved_cmd = xstrdup (cmd);
> > > -  saved_cmd_cleanup_chain = make_cleanup (xfree, (void *) saved_cmd);
> > > +  old_chain = make_cleanup (xfree, saved_cmd);
> > >    while (tidlist < cmd)
> > >      {
> > >        struct thread_info *tp;
> > >  <at>  <at>  -926,26 +1098,24  <at>  <at>  thread_apply_command (char *tidlist, int
> > >  	    warning (_("Thread %d has terminated."), start);
> > >  	  else
> > >  	    {
> > > +	      make_cleanup_restore_current_thread ();
> > > +

(Continue reading)

headers

Pedro Alves | 2 Jul 2008 05:28

Re: [non-stop] 10/10 split user/internal threads

A Tuesday 01 July 2008 15:03:11, Daniel Jacobowitz wrote:
> On Tue, Jul 01, 2008 at 02:37:44PM +0100, Pedro Alves wrote:
> > A Tuesday 01 July 2008 01:51:28, Daniel Jacobowitz wrote:
> > > On Mon, Jun 30, 2008 at 01:08:48AM +0100, Pedro Alves wrote:
> > There's one issue that I'd like to point out.  That is, is a
> > thread exits, and it was inferior_ptid, we can't just switch
> > inferior_ptid to null_ptid as I could with split user/internal
> > threads split.  A *lot* of things break.
> >
> > The issue arises with the OS quickly reusing ptids:
> >
> >   - inferior_ptid (ptid1) exits, we can't delete it from the
> >     thread list yet (things break, e.g. context switching...,
> >     but many other things break in target code.)
> >   - We mark it with THREAD_EXITED, but leave it there.
> >   - target notifies new thread with ptid1.  There's already
> >     such a thread in the list, and it's also the
> >     current thread -- inferior_ptid.
> >   - To add this new thread to the list, we must get rid
> >     of the old exited thread.  Conceptually, this new thread
> >     although it has the target identifier, it should have a
> >     new GDB thread id.  So, we could say that in this case,
> >     the "non-stop doesn't change threads" premise breaks.  But,
> >     then again, the user couldn't do anything to the exited
> >     thread anyway, and it can only exit is it is running, in which
> >     case the user also couldn't do most things with it.  Maybe we
> >     can just live with this exception.
>
> IMO, as long as GDB is not likely to crash, I don't think there's a
> big problem here.  If we, for instance, fail to report one pair of

(Continue reading)