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

[Pedro Alves <pedro at codesourcery dot com>]

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.

>
> > @@ -163,6 +223,10 @@ 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)
> >  {
> > @@ -177,12 +241,35 @@ 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.  */
>
> refcount

Fixed.


> > +  count = frame_level;
> > +  frame = find_relative_frame (get_current_frame (), &count);
> > +  if (count == 0
> > +      && frame != NULL
> > +      /* Either the frame ids match, of they're both invalid.
> > +	 The later case is not failsafe, but since it's highly
> > +	 unlikelly the search by level finds the wrong frame,
> > +		 it's 99.9(9)% of the times (for all practical
> > +		 purposes) safe.  */
> > +      && (frame_id_eq (get_frame_id (frame), a_frame_id)
> > +	  /* Note: could be better to check every frame_id
> > +	     member for equality here.  */
> > +	  || (!frame_id_p (get_frame_id (frame))
> > +	      && !frame_id_p (a_frame_id))))
>
> Indentation, spelling; "latter" and "unlikely" and "of the time".

Fixed.

>
> > @@ -877,18 +1057,10 @@ 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;
> > @@ -926,26 +1098,24 @@ thread_apply_command (char *tidlist, int
> >  	    warning (_("Thread %d has terminated."), start);
> >  	  else
> >  	    {
> > +	      make_cleanup_restore_current_thread ();
> > +
> >  	      if (non_stop)
> >  		context_switch_to (tp->ptid);
> >  	      else
> >  		switch_to_thread (tp->ptid);
> > +
> >  	      printf_filtered (_("\nThread %d (%s):\n"), tp->num,
> >  			       target_tid_to_str (inferior_ptid));
> >  	      execute_command (cmd, from_tty);
> > -	      strcpy (cmd, saved_cmd);	/* Restore exact command used previously
> > */ +
> > +	      /* Restore exact command used previously.  */
> > +	      strcpy (cmd, saved_cmd);
> >  	    }
> >  	}
> >      }
> >
> > -  if (!ptid_equal (current_ptid, inferior_ptid))
> > -    thread_has_changed = 1;
> > -
> > -  do_cleanups (saved_cmd_cleanup_chain);
> >    do_cleanups (old_chain);
> > -  /* Print stack frame only if we changed thread.  */
> > -  if (thread_has_changed)
> > -    print_stack_frame (get_current_frame (), 1, SRC_LINE);
> >  }
> >
> >  /* Switch to the specified thread.  Will dispatch off to
> > thread_apply_command
>
> You've moved creation of the cleanup into the loop, but not moved the
> call to do_cleanups.  Do we ever need more than one cleanup for a
> thread apply command?
>

Ooops, sorry, I missed this, and committed without taking care of it.  Let
me come back to it in a sec, should be a minor change.

> > +  /* In non-stop mode, we don't want GDB to switch threads on the
> > +     user's back, to avoid races where the user is typing a command to
> > +     apply to thread x, but GDB switches to thread y before the user
> > +     finishes entering the command.  */
>
> behind the user's back

Fixed.

> Otherwise, OK.

Thanks!  I've checked it in.

Next up, docs.

-- 
Pedro Alves