Re: [PATCH] gdbserver: Reset current_thread when its process is removed.

[Pedro Alves <palves at redhat dot com>]

On 10/15/2015 03:36 PM, Aleksandar Ristovski wrote:
> In case of running gdbserver with --multi, if a nat file
> removes a process, current_thread may remain set to now
> freed 'process' entry.
> 
> This may lead to wrong operation or a crash.
> 
> gdb/gdbserver/ChangeLog:
> 	* inferiors.c (remove_process): Reset current_thread if its
> 	associated process gets removed.
> ---
>  gdb/gdbserver/inferiors.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
> index 21f45fa..4688a44 100644
> --- a/gdb/gdbserver/inferiors.c
> +++ b/gdb/gdbserver/inferiors.c
> @@ -291,6 +291,11 @@ remove_process (struct process_info *process)
>  {
>    clear_symbol_cache (&process->symbol_cache);
>    free_all_breakpoints (process);
> +  if (current_thread && get_thread_process (current_thread) == process)
> +    {
> +      remove_thread (current_thread);
> +      current_thread = NULL;
> +    }

This seems very papering-over-something-else.

- I could argue that current_thread = NULL would be better done
inside remove_thread than here, since what you say would happen
as well if the current thread is removed, even without removing
the process.

- And then, if we remove the current thread, why not remove others?

AFAICS, other targets remove threads from within target_mourn, as that
way they have a chance of clearing auxiliary info associated with
the threads (the inferior_target_data()), and then some call
clear_inferiors, which also clears current_thread.

>    remove_inferior (&all_processes, &process->entry);
>    free (process);
>  }
> 

Thanks,
Pedro Alves