Re: Updated patch for Bug 13217 - thread apply all detach throws a SEGFAULT

[Pedro Alves <palves at redhat dot com>]

On 12/11/2012 03:36 PM, ali_anwar wrote:
> +  if (thread_count ())
> +    {
> +      struct thread_info *tp_array;
> +      struct thread_info *tp;
> +      int i, k;
> +
> +      /* Save a copy of the thread_list in case we execute detach
> +         command.  */
> +      tp_array =  xmalloc (sizeof (struct thread_info) * thread_count ());

No need to compute the thread count twice, you can cache it.  No need to
copy the whole thread structure.  Make this an array of a thread
pointers, and then,

> +      for (i = 0, tp = thread_list; tp; i++, tp = tp->next)
> +        tp_array[i] = *tp;

      ALL_THREADS (tp)
        {
          tp_array[i] = tp;
          tp->refcount++;
        }

This increments the refcount of each current thread, so that attempts to
delete it just mark it as deleted (so the C object remains valid).

> +
> +      for (k = 0; k != i; k++)
> +        if (thread_alive (&tp_array[k]))

and then write:

      for (k = 0; k != i; k++)
        {
          if (thread_alive (tp_array[k]))
            {
              switch_to_thread (tp_array[k]->ptid);

              printf_filtered (_("\nThread %d (%s):\n"),
                               (tp_array->num, target_pid_to_str (inferior_ptid));
              execute_command (cmd, from_tty);
              strcpy (cmd, saved_cmd);        /* Restore exact command used
                                                 previously.  */
            }
        }

And put this in a cleanup:

      for (k = 0; k != i; k++)
        tp_array[k]->refcount--;

So that if the command throws an error, we still leave with the correct
refcounts.

The advantages are:

 - less memory necessary for the array.
 - handles the corner case of the target reusing a ptid (see
   add_thread_silent).  IOW, this way, even if the command happens to
   make the target reuse a ptid, "thread apply all" won't run the command
   on that new threads my mistake.



> +          {
> +            switch_to_thread ((&tp_array[k])->ptid);
> +
> +            printf_filtered (_("\nThread %d (%s):\n"),
> +                             (&tp_array[k])->num, target_pid_to_str (inferior_ptid));
> +            execute_command (cmd, from_tty);
> +            strcpy (cmd, saved_cmd);        /* Restore exact command used
> +                                               previously.  */
> +           }
> +       xfree (tp_array);

This xfree should be on the cleanup as well.

> +    }
>    do_cleanups (old_chain);

-- 
Pedro Alves