This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFC]: fix for recycled thread ids


The following patch fixes a problem when a user application creates a thread shortly after another thread has completed. For nptl, thread ids are addresses. If a thread completes/dies, the tid is available for reuse by a new thread.

On RH9 and RHEL3, nptl threads do not have exit events associated with them. I have already discussed this with Daniel J. who feels that the kernels are not doing the right thing, but regardless, the current and previous RH nptl kernels are behaving this way and gdb needs to handle it. As such, when a new thread is created, if it is reusing the tid of a previous thread that gdb hasn't figured out isn't around any more, gdb ignores the create event and the new thread is not added. Ignoring the event is done because it is possible for gdb to find out about the thread before it's creation event is reported and so the create event can be redundant information.

The following fix removes the problem by also checking if the original lwp given to the old thread in the list matches the lwp for the new thread. IIRC, there is at least one platform that allows threads to change their lwps dynamically. I do not believe that platform uses the thread-db layer, but to be safe, I did not use a direct compare between the original lwp and the new lwp. Instead, I added a target vector routine that by default never returns an unequal comparison. For linux, the comparison routine points to a new routine in lin-lwp.c which does the expected comparison. The comparison is based on C compare routines whereby 0 means equal and non-zero means unequal.

Now, if a thread id for a new thread is found to already be on the list, the target comparison is made between the original lwp stored for the old thread and the lwp for the new thread. If the comparison is unequal, the old thread is deleted and then the new thread is added. Otherwise, the new thread event is ignored as before.

Ok to commit?

-- Jeff J.

2004-03-18 Jeff Johnston <jjohnstn@redhat.com>

        * thread-db.c (struct private_thread_info): Add orig_lwp field.
        (attach_thread): Save original lwp for thread after attaching.
        (check_event): When a create event occurs and the thread is already
        in the thread list, check if the old thread has gone away and the
        thread id is being recycled.
        * target.h (struct target_ops): Add to_thread_compare function.
        * target.c (update_current_target): Add to_thread_compare support.
        (debug_to_thread_compare): New function.
        (setup_target_debug): Add debug_to_thread_compare.
        * lin-lwp.c (lin_lwp_thread_compare): New function.
        (init_lin_lwp_ops): Add lin_lwp_thread_compare.

Index: thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/thread-db.c,v
retrieving revision 1.37
diff -u -p -r1.37 thread-db.c
--- thread-db.c	29 Feb 2004 02:39:47 -0000	1.37
+++ thread-db.c	18 Mar 2004 23:14:01 -0000
@@ -156,6 +156,8 @@ struct private_thread_info
 
   td_thrhandle_t th;
   td_thrinfo_t ti;
+
+  lwpid_t orig_lwp;
 };
 
 
@@ -714,6 +716,9 @@ attach_thread (ptid_t ptid, const td_thr
   ATTACH_LWP (BUILD_LWP (ti_p->ti_lid, GET_PID (ptid)), 0);
 #endif
 
+  /* Save original lwp for future reference.  */
+  tp->private->orig_lwp = ti_p->ti_lid;
+
   /* Enable thread event reporting for this thread.  */
   err = td_thr_event_enable_p (th_p, 1);
   if (err != TD_OK)
@@ -850,8 +855,35 @@ check_event (ptid_t ptid)
 
 	  /* We may already know about this thread, for instance when the
 	     user has issued the `info threads' command before the SIGTRAP
-	     for hitting the thread creation breakpoint was reported.  */
-	  if (!in_thread_list (ptid))
+	     for hitting the thread creation breakpoint was reported.
+	     However, we have a problem when threads die and new threads
+	     are created shortly after.
+
+	     If a thread dies and gdb is unaware the thread has died,
+	     a new thread could reuse the tid that is no longer being
+	     used.  Thus, if we find the new tid is already in our list,
+	     we must punt to the lower level to tell us if the new thread
+	     and old thread are one and the same.  If they are different,
+	     the old thread must have gone away and the tid is being
+	     recycled.
+
+	     If we know the original thread has gone away, we can
+	     delete the previous entry with the tid in the thread list and then
+	     attach the new thread.  Otherwise, we ignore the create event.  */
+
+	  if (in_thread_list (ptid))
+	    {
+	      struct thread_info *t = find_thread_pid (ptid);
+	      if (target_beneath->to_thread_compare (BUILD_LWP (t->private->orig_lwp, 
+								GET_PID (ptid)), 
+						     BUILD_LWP (ti.ti_lid, 
+								GET_PID (ptid))))
+		{
+		  delete_thread (ptid);
+		  attach_thread (ptid, msg.th_p, &ti, 1);
+		}
+	    }
+	  else
 	    attach_thread (ptid, msg.th_p, &ti, 1);
 
 	  break;
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.57
diff -u -p -r1.57 target.h
--- target.h	4 Feb 2004 21:49:55 -0000	1.57
+++ target.h	18 Mar 2004 23:14:01 -0000
@@ -369,6 +369,7 @@ struct target_ops
     int (*to_can_run) (void);
     void (*to_notice_signals) (ptid_t ptid);
     int (*to_thread_alive) (ptid_t ptid);
+    int (*to_thread_compare) (ptid_t ptid1, ptid_t ptid2);
     void (*to_find_new_threads) (void);
     char *(*to_pid_to_str) (ptid_t);
     char *(*to_extra_thread_info) (struct thread_info *);
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.71
diff -u -p -r1.71 target.c
--- target.c	9 Mar 2004 16:16:52 -0000	1.71
+++ target.c	18 Mar 2004 23:14:01 -0000
@@ -161,6 +161,8 @@ static void debug_to_notice_signals (pti
 
 static int debug_to_thread_alive (ptid_t);
 
+static int debug_to_thread_compare (ptid_t, ptid_t);
+
 static void debug_to_stop (void);
 
 /* Pointer to array of target architecture structures; the size of the
@@ -424,6 +426,7 @@ update_current_target (void)
       INHERIT (to_can_run, t);
       INHERIT (to_notice_signals, t);
       INHERIT (to_thread_alive, t);
+      INHERIT (to_thread_compare, t);
       INHERIT (to_find_new_threads, t);
       INHERIT (to_pid_to_str, t);
       INHERIT (to_extra_thread_info, t);
@@ -601,6 +604,9 @@ update_current_target (void)
   de_fault (to_thread_alive, 
 	    (int (*) (ptid_t)) 
 	    return_zero);
+  de_fault (to_thread_compare, 
+	    (int (*) (ptid_t, ptid_t)) 
+	    return_zero);
   de_fault (to_find_new_threads, 
 	    (void (*) (void)) 
 	    target_ignore);
@@ -2263,6 +2269,19 @@ debug_to_thread_alive (ptid_t ptid)
   return retval;
 }
 
+static int
+debug_to_thread_compare (ptid_t ptid1, ptid_t ptid2)
+{
+  int retval;
+
+  retval = debug_target.to_thread_compare (ptid1, ptid2);
+
+  fprintf_unfiltered (gdb_stdlog, "target_thread_compare (%ld, %ld) = %d\n",
+		      TIDGET (ptid1), TIDGET (ptid2), retval);
+
+  return retval;
+}
+
 static void
 debug_to_find_new_threads (void)
 {
@@ -2393,6 +2412,7 @@ setup_target_debug (void)
   current_target.to_can_run = debug_to_can_run;
   current_target.to_notice_signals = debug_to_notice_signals;
   current_target.to_thread_alive = debug_to_thread_alive;
+  current_target.to_thread_compare = debug_to_thread_compare;
   current_target.to_find_new_threads = debug_to_find_new_threads;
   current_target.to_stop = debug_to_stop;
   current_target.to_xfer_partial = debug_to_xfer_partial;
Index: lin-lwp.c
===================================================================
RCS file: /cvs/src/src/gdb/lin-lwp.c,v
retrieving revision 1.52
diff -u -p -r1.52 lin-lwp.c
--- lin-lwp.c	8 Oct 2003 20:05:56 -0000	1.52
+++ lin-lwp.c	18 Mar 2004 23:14:01 -0000
@@ -1728,6 +1728,13 @@ lin_lwp_thread_alive (ptid_t ptid)
   return 1;
 }
 
+/* Compare thread lwps and return 0 if equal, non-zero if not equal.  */
+static int
+lin_lwp_thread_compare (ptid_t old, ptid_t new)
+{
+  return (GET_LWP (old) != GET_LWP (new));
+}
+
 static char *
 lin_lwp_pid_to_str (ptid_t ptid)
 {
@@ -1764,6 +1771,7 @@ init_lin_lwp_ops (void)
   lin_lwp_ops.to_create_inferior = lin_lwp_create_inferior;
   lin_lwp_ops.to_mourn_inferior = lin_lwp_mourn_inferior;
   lin_lwp_ops.to_thread_alive = lin_lwp_thread_alive;
+  lin_lwp_ops.to_thread_compare = lin_lwp_thread_compare;
   lin_lwp_ops.to_pid_to_str = lin_lwp_pid_to_str;
   lin_lwp_ops.to_post_startup_inferior = child_post_startup_inferior;
   lin_lwp_ops.to_post_attach = child_post_attach;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]