From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11700 invoked by alias); 4 Jun 2004 21:28:53 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 11685 invoked from network); 4 Jun 2004 21:28:52 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 4 Jun 2004 21:28:52 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.10/8.12.10) with ESMTP id i54LSpi5014042 for ; Fri, 4 Jun 2004 17:28:51 -0400 Received: from pobox.toronto.redhat.com (pobox.toronto.redhat.com [172.16.14.4]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i54LSp005140; Fri, 4 Jun 2004 17:28:51 -0400 Received: from touchme.toronto.redhat.com (IDENT:postfix@touchme.toronto.redhat.com [172.16.14.9]) by pobox.toronto.redhat.com (8.12.8/8.12.8) with ESMTP id i54LSptn003627; Fri, 4 Jun 2004 17:28:51 -0400 Received: from redhat.com (toocool.toronto.redhat.com [172.16.14.72]) by touchme.toronto.redhat.com (Postfix) with ESMTP id 0F5B980019B; Fri, 4 Jun 2004 17:28:51 -0400 (EDT) Message-ID: <40C0E992.5020902@redhat.com> Date: Fri, 04 Jun 2004 21:28:00 -0000 From: Jeff Johnston User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030624 Netscape/7.1 MIME-Version: 1.0 To: Michael Snyder Cc: gdb-patches@sources.redhat.com, drow@false.org Subject: Re: [RFA]: Dead Thread Patch References: <40C0BCB5.8090407@redhat.com> <40C0CBF7.1070300@redhat.com> In-Reply-To: <40C0CBF7.1070300@redhat.com> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2004-06/txt/msg00082.txt.bz2 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 >> >> * 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 > > > >