Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Yao Qi <qiyaoltc@gmail.com>
Cc: gdb-patches@sourceware.org
Subject: [pushed] GC gdb/thread.c:current_thread_cleanup_chain (Re: [PATCH 2/2] Don't delete thread_info if refcount isn't zero)
Date: Mon, 10 Apr 2017 14:50:00 -0000	[thread overview]
Message-ID: <0a7fb2a3-4aa3-bd96-7306-4e4ac509fa89@redhat.com> (raw)
In-Reply-To: <861st0l6st.fsf@gmail.com>

On 04/10/2017 02:40 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> I'm not sure what you mean by "wrong".  I'm saying that the old comment
>> still makes sense.  That comment is talking about the 
>> "find_inferior_ptid (old->thread->ptid) != NULL" line, which is a 
>> lookup for an _inferior_ not a thread.  Both the inferior and thread_info
> 
> I thought it is about a thread.
> 
>> object are still around, but the process may have exited/been detached
>> meanwhile, and consequently the inferior's "pid" field is now
>> zero.  And in that case, we don't restore back the selected thread.
> 
> I move the comment close to "find_inferior_ptid (old->thread->ptid) != NULL"
> line, to make it clearer.  Patch below is pushed in.

Thanks!

I noticed that ...

> -/* A thread_ptid_changed observer.  Update all currently installed
> -   current_thread_cleanup cleanups that want to switch back to
> -   OLD_PTID to switch back to NEW_PTID instead.  */
> -
> -static void
> -restore_current_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
> -{
> -  struct current_thread_cleanup *it;
> -
> -  for (it = current_thread_cleanup_chain; it != NULL; it = it->next)
> -    {
> -      if (ptid_equal (it->inferior_ptid, old_ptid))
> -	it->inferior_ptid = new_ptid;
> -    }
> -}


... this means current_thread_cleanup_chain is no longer useful.

So I pushed in the patch below.

From 996812e3d43f78b17b6454d2948cd825ec98c63b Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 10 Apr 2017 15:18:49 +0100
Subject: [PATCH] GC gdb/thread.c:current_thread_cleanup_chain

Commit 803bdfe43083475c7df3db38dc96f4e20d05457d ("Don't delete
thread_info if refcount isn't zero") eliminated
restore_current_thread_ptid_changed, so current_thread_cleanup_chain
is no longer necessary either.

gdb/ChangeLog:
2017-04-10  Pedro Alves  <palves@redhat.com>

	* thread.c (struct current_thread_cleanup) <next>: Delete field.
	(current_thread_cleanup_chain): Delete.
	(restore_current_thread_cleanup_dtor)
	(make_cleanup_restore_current_thread): Remove references to
	current_thread_cleanup_chain.
---
 gdb/ChangeLog |  8 ++++++++
 gdb/thread.c  | 17 -----------------
 2 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3cb6cd7..5e7736e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2017-04-10  Pedro Alves  <palves@redhat.com>
+
+	* thread.c (struct current_thread_cleanup) <next>: Delete field.
+	(current_thread_cleanup_chain): Delete.
+	(restore_current_thread_cleanup_dtor)
+	(make_cleanup_restore_current_thread): Remove references to
+	current_thread_cleanup_chain.
+
 2017-04-10  Alan Hayward  <alan.hayward@arm.com>
 
 	* msp430-tdep.c (msp430_pseudo_register_read): Never return
diff --git a/gdb/thread.c b/gdb/thread.c
index 2d936cd..3aca307 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1549,11 +1549,6 @@ restore_selected_frame (struct frame_id a_frame_id, int frame_level)
 
 struct current_thread_cleanup
 {
-  /* Next in list of currently installed 'struct
-     current_thread_cleanup' cleanups.  See
-     'current_thread_cleanup_chain' below.  */
-  struct current_thread_cleanup *next;
-
   thread_info *thread;
   struct frame_id selected_frame_id;
   int selected_frame_level;
@@ -1562,13 +1557,6 @@ struct current_thread_cleanup
   int was_removable;
 };
 
-/* A chain of currently installed 'struct current_thread_cleanup'
-   cleanups.  Restoring the previously selected thread looks up the
-   old thread in the thread list by ptid.  If the thread changes ptid,
-   we need to update the cleanup's thread structure so the look up
-   succeeds.  */
-static struct current_thread_cleanup *current_thread_cleanup_chain;
-
 static void
 do_restore_current_thread_cleanup (void *arg)
 {
@@ -1609,8 +1597,6 @@ restore_current_thread_cleanup_dtor (void *arg)
   struct thread_info *tp;
   struct inferior *inf;
 
-  current_thread_cleanup_chain = current_thread_cleanup_chain->next;
-
   if (old->thread != NULL)
     old->thread->decref ();
 
@@ -1642,9 +1628,6 @@ make_cleanup_restore_current_thread (void)
   old->inf_id = current_inferior ()->num;
   old->was_removable = current_inferior ()->removable;
 
-  old->next = current_thread_cleanup_chain;
-  current_thread_cleanup_chain = old;
-
   if (!ptid_equal (inferior_ptid, null_ptid))
     {
       struct frame_info *frame;
-- 
2.5.5



  reply	other threads:[~2017-04-10 14:50 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-28  8:24 [PATCH 0/2] Fix thread_info refcount Yao Qi
2017-03-28  8:24 ` [PATCH 2/2] Delete thread_info is refcount is zero Yao Qi
2017-03-28 12:10   ` Pedro Alves
2017-04-05 21:15     ` [PATCH 0/2] Don't delete thread_info if refcount isn't zero Yao Qi
2017-04-05 21:15       ` [PATCH 1/2] Hoist code on marking thread as exited Yao Qi
2017-04-06  9:27         ` Pedro Alves
2017-04-05 21:15       ` [PATCH 2/2] Don't delete thread_info if refcount isn't zero Yao Qi
2017-04-06 10:18         ` Pedro Alves
2017-04-07  9:22           ` Yao Qi
2017-04-07 10:29             ` Pedro Alves
2017-04-10 13:40               ` Yao Qi
2017-04-10 14:50                 ` Pedro Alves [this message]
2017-04-10 15:57           ` Pedro Alves
     [not found] ` <1490689483-16084-2-git-send-email-yao.qi@linaro.org>
2017-03-28 12:12   ` [PATCH 1/2] Add constructor and destructor to thread_info Pedro Alves
2017-03-28 14:08     ` Yao Qi
2017-03-28 14:47       ` Pedro Alves
2017-03-28 15:40         ` Yao Qi
2017-03-28 22:35           ` Pedro Alves
2017-03-29 15:59             ` Yao Qi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0a7fb2a3-4aa3-bd96-7306-4e4ac509fa89@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=qiyaoltc@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox