From patchwork Wed Mar 25 00:10:43 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 5803 Received: (qmail 22674 invoked by alias); 25 Mar 2015 00:10:48 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 22660 invoked by uid 89); 25 Mar 2015 00:10:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 25 Mar 2015 00:10:46 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t2P0AjL8000668 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Tue, 24 Mar 2015 20:10:45 -0400 Received: from brno.lan (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t2P0AioT011651 for ; Tue, 24 Mar 2015 20:10:44 -0400 From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH] update thread list, delete exited threads Date: Wed, 25 Mar 2015 00:10:43 +0000 Message-Id: <1427242243-419-1-git-send-email-palves@redhat.com> On GNU/Linux, if the running kernel supports clone events, then linux-thread-db.c defers thread listing to the target beneath: static void thread_db_update_thread_list (struct target_ops *ops) { ... if (target_has_execution && !thread_db_use_events ()) ops->beneath->to_update_thread_list (ops->beneath); else thread_db_update_thread_list_td_ta_thr_iter (ops); ... } However, when live debugging, the target beneath, linux-nat.c, does not implement the to_update_thread_list method. The result is that if a thread is marked exited (because it can't be deleted right now, e.g., it was the selected thread), then it won't ever be deleted, until the process exits or is killed/detached. A similar thing happens with the remote.c target. Because its target_update_thread_list implementation skips exited threads when it walks the current thread list looking for threads that no longer exits on the target side, using ALL_NON_EXITED_THREADS_SAFE, stale exited threads are never deleted. This is not a big deal -- I can't think of any way this might be user visible, other than gdb's memory growing a tiny bit whenever a thread gets stuck in exited state. Still, might as well clean things up properly. All other targets use prune_threads, so are unaffected. The fix adds a ALL_THREADS_SAFE macro, that like ALL_NON_EXITED_THREADS_SAFE, walks the thread list and allows deleting the iterated thread, and uses that in places that are walking the thread list in order to delete threads. Actually, after converting linux-nat.c and remote.c to use this, we find the only other user of ALL_NON_EXITED_THREADS_SAFE is also walking the list to delete threads. So we convert that too, and end up deleting ALL_NON_EXITED_THREADS_SAFE. Tested on x86_64 Fedora 20, native and gdbserver. gdb/ChangeLog 2015-03-24 Pedro Alves * gdbthread.h (ALL_NON_EXITED_THREADS_SAFE): Rename to ... (ALL_THREADS_SAFE): ... this, and don't skip exited threads. (delete_exited_threads): New declaration. * infrun.c (follow_exec): Use ALL_THREADS_SAFE. * linux-nat.c (linux_nat_update_thread_list): New function. (linux_nat_add_target): Install it. * remote.c (remote_update_thread_list): Use ALL_THREADS_SAFE. * thread.c (prune_threads): Use ALL_THREADS_SAFE. (delete_exited_threads): New function. --- gdb/gdbthread.h | 14 +++++++++----- gdb/infrun.c | 2 +- gdb/linux-nat.c | 18 ++++++++++++++++++ gdb/remote.c | 2 +- gdb/thread.c | 19 ++++++++++++++++--- 5 files changed, 45 insertions(+), 10 deletions(-) diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h index bb15717..ff7cec2 100644 --- a/gdb/gdbthread.h +++ b/gdb/gdbthread.h @@ -380,13 +380,12 @@ extern struct thread_info *iterate_over_threads (thread_callback_func, void *); for (T = thread_list; T; T = T->next) \ if ((T)->state != THREAD_EXITED) -/* Like ALL_NON_EXITED_THREADS, but allows deleting the currently - iterated thread. */ -#define ALL_NON_EXITED_THREADS_SAFE(T, TMP) \ +/* Traverse all threads, including those that have THREAD_EXITED + state. Allows deleting the currently iterated thread. */ +#define ALL_THREADS_SAFE(T, TMP) \ for ((T) = thread_list; \ (T) != NULL ? ((TMP) = (T)->next, 1): 0; \ - (T) = (TMP)) \ - if ((T)->state != THREAD_EXITED) + (T) = (TMP)) extern int thread_count (void); @@ -484,6 +483,11 @@ extern void update_thread_list (void); extern void prune_threads (void); +/* Delete threads marked THREAD_EXITED. Unlike prune_threads, this + does not consult the target about whether the thread is alive right + now. */ +extern void delete_exited_threads (void); + /* Return true if PC is in the stepping range of THREAD. */ int pc_in_thread_step_range (CORE_ADDR pc, struct thread_info *thread); diff --git a/gdb/infrun.c b/gdb/infrun.c index 4114246..4586aa9 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1099,7 +1099,7 @@ follow_exec (ptid_t ptid, char *execd_pathname) them. Deleting them now rather than at the next user-visible stop provides a nicer sequence of events for user and MI notifications. */ - ALL_NON_EXITED_THREADS_SAFE (th, tmp) + ALL_THREADS_SAFE (th, tmp) if (ptid_get_pid (th->ptid) == pid && !ptid_equal (th->ptid, ptid)) delete_thread (th->ptid); diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index 660dc8f..af68617 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -4027,6 +4027,23 @@ linux_nat_thread_alive (struct target_ops *ops, ptid_t ptid) return linux_thread_alive (ptid); } +/* Implement the to_update_thread_list target method for this + target. */ + +static void +linux_nat_update_thread_list (struct target_ops *ops) +{ + if (linux_supports_traceclone ()) + { + /* With support for clone events, we add/delete threads from the + list as clone/exit events are processed, so just try deleting + exited threads still in the thread list. */ + delete_exited_threads (); + } + else + prune_threads (); +} + static char * linux_nat_pid_to_str (struct target_ops *ops, ptid_t ptid) { @@ -4862,6 +4879,7 @@ linux_nat_add_target (struct target_ops *t) t->to_kill = linux_nat_kill; t->to_mourn_inferior = linux_nat_mourn_inferior; t->to_thread_alive = linux_nat_thread_alive; + t->to_update_thread_list = linux_nat_update_thread_list; t->to_pid_to_str = linux_nat_pid_to_str; t->to_thread_name = linux_nat_thread_name; t->to_has_thread_control = tc_schedlock; diff --git a/gdb/remote.c b/gdb/remote.c index fd677fe..b491f42 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -2844,7 +2844,7 @@ remote_update_thread_list (struct target_ops *ops) /* CONTEXT now holds the current thread list on the remote target end. Delete GDB-side threads no longer found on the target. */ - ALL_NON_EXITED_THREADS_SAFE (tp, tmp) + ALL_THREADS_SAFE (tp, tmp) { for (i = 0; VEC_iterate (thread_item_t, context.items, i, item); diff --git a/gdb/thread.c b/gdb/thread.c index ec398f5..80c8705 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -625,16 +625,29 @@ thread_alive (struct thread_info *tp) void prune_threads (void) { - struct thread_info *tp, *next; + struct thread_info *tp, *tmp; - for (tp = thread_list; tp; tp = next) + ALL_THREADS_SAFE (tp, tmp) { - next = tp->next; if (!thread_alive (tp)) delete_thread (tp->ptid); } } +/* See gdbthreads.h. */ + +void +delete_exited_threads (void) +{ + struct thread_info *tp, *tmp; + + ALL_THREADS_SAFE (tp, tmp) + { + if (tp->state == THREAD_EXITED) + delete_thread (tp->ptid); + } +} + /* Disable storing stack temporaries for the thread whose id is stored in DATA. */