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

Pedro Alves
2008-07-11T11:16:41Z

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