Re: [patch] Fix internal-error on dead LWPs with no associated thread

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Mon, 29 Jun 2009 20:41:51 +0200, Pedro Alves wrote:
>  1 - I'm not convinced currently that adding threads immediately to the list in
>      all-stop mode in linux_handle_extended_wait is a good idea.  See here for
>      thoughts around that:
> 
>       http://sourceware.org/ml/gdb/2009-05/msg00067.html

I do not share this opinion - not attaching short-lived threads should not be
allowed for performance reasons.  When such short-lived thread crashes GDB
should catch such crash.


>  2 - If the target has let the thread escape this far without having added it to
>      the list, *and* the target needs to book-keep extra thread info associated
>      with the thread structure, than your patch looks like paparing over a bug.
>      It's just a simple to handle it in the target's target_wait implementation,
>      just before returning an event.
> 
>  3 - I really just meant to just remove this whole block:
> 
> -  if (ecs->new_thread_event)
> -    {
> -      if (non_stop)
> -	/* Non-stop assumes that the target handles adding new threads
> -	   to the thread list.  */
> -	internal_error (__FILE__, __LINE__, "\
> -targets should add new threads to the thread list themselves in non-stop mode.");
> -
> -      /* We may want to consider not doing a resume here in order to
> -	 give the user a chance to play with the new thread.  It might
> -	 be good to make that a user-settable option.  */
> -
> -      /* At this point, all threads are stopped (happens automatically
> -	 in either the OS or the native code).  Therefore we need to
> -	 continue all threads in order to make progress.  */
> -
> -      if (!ptid_equal (ecs->ptid, inferior_ptid))
> -	context_switch (ecs->ptid);
> -      target_resume (RESUME_ALL, 0, TARGET_SIGNAL_0);
> -      prepare_to_wait (ecs);
> -      return;
> -    }

Such a review for myself why:

On GNU/Linux if linux_test_for_tracefork() fails then the new LWP is left
unstopped - so it needs no target_resume.

On GNU/Linux if linux_test_for_tracefork() succeeds then the new LWP is left
unstopped immediately after catching its clone event by
linux_handle_extended_wait as it is called with STOPPING 0.  So no
target_resume is needed.

For other OSes expectin no one stops the new LWP.

(I currently do not have an opinion on this removal, it is just a dead code.)


Thanks,
Jan