Re: [PATCH 8/8] Fix removing inferiors from within "thread apply" commands

[Yao Qi <qiyaoltc at gmail dot com>]

Pedro Alves <palves@redhat.com> writes:

> See patch below that implements this.  I also added comments
> explaining the thread_info and inferior objects' refcount handling
> to hopefully make things clearer.
>

Yes, they are helpful.

> I then realized that unlike thread_info objects, there's a
> "current inferior" pointer, and switching the current
> inferior always goes through a single function (set_current_inferior),
> so we can make being the current inferior be a strong reference
> accounted for in the inferior's refcount.  This simplifies the

It is reasonable to me.

> inferior::deletable method's implementation which no longer
> compares this with current_inferior().  It's just a trade off,
> because now set_current_inferior and initialize_inferiors
> must manage the refcounts.  The end result is just the same.
> Not sure which version is clearer -- this, or keeping inferior::deletable
> as it was (and reverting the set_current_inferior change).
> Maybe the other way is a little bit more efficient given
> deleting inferiors is way less frequently done than switching
> around the current inferior.  OTOH maybe this way is clearer.
> If you have a preference, let me know.

This version is good to me.

> -struct thread_info
> +/* Threads are intrusively refcounted objects.  Being the
> +   user-selected thread is normally considered an implicit strong
> +   reference and is thus not accounted in the refcount, unlike
> +   inferior objects.  This is necessary, because there's no "current
> +   thread" pointer.  Instead the current thread is inferred from the
> +   inferior_ptid global.  However, when GDB needs to remember the
> +   selected thread to later restore it, GDB bumps the thread object's
> +   refcount, to prevent something deleting the thread object before
> +   reverting back (e.g., due to a "kill" command.  If the thread

Missing ")"?

-- 
Yao (齐尧)