Re: [RFA]: Dead Thread Patch

[Jeff Johnston <jjohnstn@redhat.com>]

Michael Snyder wrote:
> Jeff Johnston wrote:
> 
>> The attached patch handles a problem I have run into while working on 
>> threaded watchpoints.  What happens is that a thread may die and we 
>> have other events occurring at the same time (e.g. watchpoints).  If 
>> the thread_alive() function gets run by a gdb interface in the 
>> meantime, the thread will be removed on the thread list.  What later 
>> happens is that we get the death event to handle and we see a tid that 
>> is not found on the thread list.  We think we have discovered a new 
>> thread and add it.  This causes all kinds of problems in the future if 
>> we attempt to get registers or switch to the dead thread, etc.
>>
>> My patch attempts to recognize the situation and avoid any processing 
>> of the dead thread event.  I have tested this with the testsuite on 
>> i686 to verify there are no regressions.
>>
>> Daniel, I cc'd you in case this fixes some of the occasional random 
>> thread errors you were seeing and couldn't explain.
>>
>> Ok to commit?
> 
> 
> Looks good to me (s/prior/previously/)
> 

Thanks Michael.  Change made and patch checked in.

-- Jeff J.

>>
>> -- Jeff J.
>>
>> 2004-06-04  Jeff Johnston  <jjohnstn@redhat.com>
>>
>>         * infrun.c (handle_inferior_event): Don't treat an invalid ptid
>>         as a new thread event.
>>         * thread_db.c (thread_get_info_callback): If the thread is a
>>         zombie, return TD_THR_ZOMBIE.
>>         * (thread_from_lwp): If thread_get_info_callback returns
>>         TD_THR_ZOMBIE, check if the thread is still on the thread list
>>         and return a -1 ptid if not found.
>>         (thread_db_wait): If thread_from_lwp returns a -1 ptid, then
>>         change the status to TARGET_WAITKIND_SPURIOUS.
>>
>>
>> ------------------------------------------------------------------------
>>
>> Index: infrun.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/infrun.c,v
>> retrieving revision 1.163
>> diff -u -p -r1.163 infrun.c
>> --- infrun.c    14 May 2004 18:45:42 -0000    1.163
>> +++ infrun.c    4 Jun 2004 18:13:53 -0000
>> @@ -1332,6 +1332,7 @@ handle_inferior_event (struct execution_
>>    /* If it's a new process, add it to the thread database */
>>  
>>    ecs->new_thread_event = (!ptid_equal (ecs->ptid, inferior_ptid)
>> +               && !ptid_equal (ecs->ptid, minus_one_ptid)
>>                 && !in_thread_list (ecs->ptid));
>>  
>>    if (ecs->ws.kind != TARGET_WAITKIND_EXITED
>> Index: thread-db.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/thread-db.c,v
>> retrieving revision 1.40
>> diff -u -p -r1.40 thread-db.c
>> --- thread-db.c    25 May 2004 14:58:31 -0000    1.40
>> +++ thread-db.c    4 Jun 2004 18:13:53 -0000
>> @@ -252,7 +252,10 @@ thread_db_state_str (td_thr_state_e stat
>>  
>>     THP is a handle to the current thread; if INFOP is not NULL, the
>>     struct thread_info associated with this thread is returned in
>> -   *INFOP.  */
>> +   *INFOP.
>> +
>> +   If the thread is a zombie, TD_THR_ZOMBIE is returned.  Otherwise,
>> +   zero is returned to indicate success.  */
>>  
>>  static int
>>  thread_get_info_callback (const td_thrhandle_t *thp, void *infop)
>> @@ -271,6 +274,16 @@ thread_get_info_callback (const td_thrha
>>    thread_ptid = BUILD_THREAD (ti.ti_tid, GET_PID (inferior_ptid));
>>    thread_info = find_thread_pid (thread_ptid);
>>  
>> +  /* In the case of a zombie thread, don't continue.  We don't want to
>> +     attach to it thinking it is a new thread and we don't want to mark
>> +     it as valid.  */
>> +  if (ti.ti_state == TD_THR_UNKNOWN || ti.ti_state == TD_THR_ZOMBIE)
>> +    {
>> +      if (infop != NULL)
>> +        *(struct thread_info **) infop = thread_info;
>> +      return TD_THR_ZOMBIE;
>> +    }
>> +
>>    if (thread_info == NULL)
>>      {
>>        /* New thread.  Attach to it now (why wait?).  */
>> @@ -355,7 +368,19 @@ thread_from_lwp (ptid_t ptid)
>>         GET_LWP (ptid), thread_db_err_str (err));
>>  
>>    thread_info = NULL;
>> -  thread_get_info_callback (&th, &thread_info);
>> +
>> +  /* Fetch the thread info.  If we get back TD_THR_ZOMBIE, then the
>> +     event thread has already died.  If another gdb interface has called
>> +     thread_alive() prior, the thread won't be found on the thread list
>> +     anymore.  In that case, we don't want to process this ptid anymore
>> +     to avoid the possibility of later treating it as a newly
>> +     discovered thread id that we should add to the list.  Thus,
>> +     we return a -1 ptid which is also how the thread list marks a
>> +     dead thread.  */
>> +  if (thread_get_info_callback (&th, &thread_info) == TD_THR_ZOMBIE
>> +      && thread_info == NULL)
>> +    return pid_to_ptid (-1);
>> +
>>    gdb_assert (thread_info && thread_info->private->ti_valid);
>>  
>>    return BUILD_THREAD (thread_info->private->ti.ti_tid, GET_PID (ptid));
>> @@ -950,7 +975,16 @@ thread_db_wait (ptid_t ptid, struct targ
>>    if (!ptid_equal (trap_ptid, null_ptid))
>>      trap_ptid = thread_from_lwp (trap_ptid);
>>  
>> -  return thread_from_lwp (ptid);
>> +  /* Change the ptid back into the higher level PID + TID format.
>> +     If the thread is dead and no longer on the thread list, we will 
>> +     get back a dead ptid.  This can occur if the thread death event
>> +     gets postponed by other simultaneous events.  In such a case, 
>> +     we want to just ignore the event and continue on.  */
>> +  ptid = thread_from_lwp (ptid);
>> +  if (GET_PID (ptid) == -1)
>> +    ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
>> +  +  return ptid;
>>  }
>>  
>>  static int
> 
> 
> 
>