This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc 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]

pthread_kill and invalid thread IDs


This patch lets pthread_kill behave more robustly when handed invalid thread
IDs.  It does a linear search of the cached stacks to find the supplied
thread descriptor; if it does, and the tid is >0, then we have a valid
descriptor (at the moment, obviously it might exit after that check).

I realize the linear search sucks.  However, I think POSIX doesn't give us a
choice here; pthread_kill is the one and only function which "must" return
ESRCH when passed an invalid thread ID (all others "may" return ESRCH).  A
defect report was raised against the standard to downgrade this to "may
fail", but it was rejected on the principle that pthread_kill (detached_id,
0) ought to be valid even if detached_id has exited.  See included testcase.

Defect report:
  http://www.opengroup.org/austin/mailarchives/ag-review/msg01348.html

Minutes with rejection:
  http://www.opengroup.org/austin/docs/austin_148.txt

-- 
Daniel Jacobowitz
CodeSourcery, LLC

2005-11-21  Daniel Jacobowitz  <dan@codesourcery.com>

	* Makefile (tests): Add tst-kill7.
	* tst-kill4.c: Doc fix.
	* tst-kill7.c: New test.
	* pthreadP.h (__find_tid_in_stack_list): New prototype.
	* pthread_create.c (__find_in_stack_list_unlocked): New function,
	broken out from __find_in_stack_list.
	(__find_in_stack_list): Use it.
	(__find_tid_in_stack_list): New function.
	* sysdeps/unix/sysv/linux/pthread_kill.c (__pthread_kill): Use
	__find_tid_in_stack_list.

Index: nptl/Makefile
===================================================================
RCS file: /big/fsf/rsync/glibc/libc/nptl/Makefile,v
retrieving revision 1.171
diff -u -p -r1.171 Makefile
--- nptl/Makefile	16 Oct 2005 09:25:52 -0000	1.171
+++ nptl/Makefile	21 Nov 2005 21:25:15 -0000
@@ -208,6 +208,7 @@ tests = tst-attr1 tst-attr2 tst-attr3 \
 	tst-align tst-align2 tst-align3 \
 	tst-basic1 tst-basic2 tst-basic3 tst-basic4 tst-basic5 tst-basic6 \
 	tst-kill1 tst-kill2 tst-kill3 tst-kill4 tst-kill5 tst-kill6 \
+	tst-kill7 \
 	tst-raise1 \
 	tst-join1 tst-join2 tst-join3 tst-join4 tst-join5 \
 	tst-detach1 \
Index: nptl/pthreadP.h
===================================================================
RCS file: /big/fsf/rsync/glibc/libc/nptl/pthreadP.h,v
retrieving revision 1.56
diff -u -p -r1.56 pthreadP.h
--- nptl/pthreadP.h	20 Nov 2004 07:14:55 -0000	1.56
+++ nptl/pthreadP.h	21 Nov 2005 21:25:15 -0000
@@ -219,6 +219,9 @@ extern struct xid_command *__xidcmd attr
 extern struct pthread *__find_in_stack_list (struct pthread *pd)
      attribute_hidden internal_function;
 
+extern pid_t __find_tid_in_stack_list (struct pthread *pd)
+     attribute_hidden internal_function;
+
 /* Deallocate a thread's stack after optionally making sure the thread
    descriptor is still valid.  */
 extern void __free_tcb (struct pthread *pd) attribute_hidden internal_function;
Index: nptl/pthread_create.c
===================================================================
RCS file: /big/fsf/rsync/glibc/libc/nptl/pthread_create.c,v
retrieving revision 1.44
diff -u -p -r1.44 pthread_create.c
--- nptl/pthread_create.c	1 Oct 2005 17:19:37 -0000	1.44
+++ nptl/pthread_create.c	21 Nov 2005 21:25:15 -0000
@@ -55,16 +55,13 @@ unsigned int __nptl_nthreads = 1;
 #include "createthread.c"
 
 
-struct pthread *
+static struct pthread *
 internal_function
-__find_in_stack_list (pd)
-     struct pthread *pd;
+__find_in_stack_list_unlocked (struct pthread *pd)
 {
   list_t *entry;
   struct pthread *result = NULL;
 
-  lll_lock (stack_cache_lock);
-
   list_for_each (entry, &stack_used)
     {
       struct pthread *curp;
@@ -90,11 +87,46 @@ __find_in_stack_list (pd)
 	  }
       }
 
+  return result;
+}
+
+struct pthread *
+internal_function
+__find_in_stack_list (pd)
+     struct pthread *pd;
+{
+  struct pthread *result;
+
+  lll_lock (stack_cache_lock);
+
+  result = __find_in_stack_list_unlocked (pd);
+
   lll_unlock (stack_cache_lock);
 
   return result;
 }
 
+pid_t
+internal_function
+__find_tid_in_stack_list (pd)
+     struct pthread *pd;
+{
+  struct pthread *result;
+  pid_t tid;
+
+  lll_lock (stack_cache_lock);
+
+  result = __find_in_stack_list_unlocked (pd);
+  if (result)
+    tid = result->tid;
+  else
+    tid = -1;
+
+  lll_unlock (stack_cache_lock);
+
+  return tid;
+}
+
 
 /* Deallocate POSIX thread-local-storage.  */
 void
Index: nptl/tst-kill4.c
===================================================================
RCS file: /big/fsf/rsync/glibc/libc/nptl/tst-kill4.c,v
retrieving revision 1.1
diff -u -p -r1.1 tst-kill4.c
--- nptl/tst-kill4.c	21 Feb 2003 20:27:54 -0000	1.1
+++ nptl/tst-kill4.c	21 Nov 2005 21:25:15 -0000
@@ -52,8 +52,8 @@ do_test (void)
      the implementation.  Namely, that the memory allocated for the
      thread descriptor is not going away, that the the TID field is
      cleared and therefore the signal is sent to process 0, and that
-     we can savely assume there is no other process with this ID at
-     that time.  */
+     we can safely assume there is no other process with this ID at
+     that time.  See tst-kill7.c for a more violent test.  */
   int e = pthread_kill (th, 0);
   if (e == 0)
     {
Index: nptl/tst-kill7.c
===================================================================
RCS file: nptl/tst-kill7.c
diff -N nptl/tst-kill7.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ nptl/tst-kill7.c	21 Nov 2005 21:25:15 -0000
@@ -0,0 +1,67 @@
+/* Copyright (C) 2005 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <errno.h>
+#include <signal.h>
+
+#define THREADS 10
+#define TOTAL 100
+
+pthread_t all_threads[THREADS];
+
+void *
+tf (void *arg)
+{
+  struct timespec delay;
+  pthread_detach (pthread_self ());
+  delay.tv_sec = 0;
+  delay.tv_nsec = 1000 * 1000 * 30;
+  nanosleep (&delay, NULL);
+  pthread_exit (0);
+  return arg;
+}
+
+int
+do_test (void)
+{
+  struct timespec delay;
+  int i, total = 0;
+
+  delay.tv_sec = 0;
+  delay.tv_nsec = 1000 * 1000 * 50;
+
+  do
+    {
+      for (i = 0; i < THREADS; i++)
+	if (total == 0 || pthread_kill (all_threads[i], 0) == ESRCH)
+	  pthread_create (&all_threads[i], NULL, tf, NULL);
+      total += i;
+      nanosleep (&delay, NULL);
+    }
+  while (total < TOTAL);
+
+  return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#define TIMEOUT 6
+#include "../test-skeleton.c"
Index: nptl/sysdeps/unix/sysv/linux/pthread_kill.c
===================================================================
RCS file: /big/fsf/rsync/glibc/libc/nptl/sysdeps/unix/sysv/linux/pthread_kill.c,v
retrieving revision 1.12
diff -u -p -r1.12 pthread_kill.c
--- nptl/sysdeps/unix/sysv/linux/pthread_kill.c	28 Sep 2004 22:22:37 -0000	1.12
+++ nptl/sysdeps/unix/sysv/linux/pthread_kill.c	21 Nov 2005 21:25:15 -0000
@@ -1,4 +1,4 @@
-/* Copyright (C) 2002, 2003, 2004 Free Software Foundation, Inc.
+/* Copyright (C) 2002, 2003, 2004, 2005 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
 
@@ -31,9 +31,12 @@ __pthread_kill (threadid, signo)
      int signo;
 {
   struct pthread *pd = (struct pthread *) threadid;
+  pid_t tid;
 
-  /* Make sure the descriptor is valid.  */
-  if (INVALID_TD_P (pd))
+  /* Make sure the descriptor is valid.  We must try harder than usual;
+     POSIX allows for invalid thread IDs to be passed to pthread_kill.  */
+  tid = __find_tid_in_stack_list (pd);
+  if (tid <= 0)
     /* Not a valid thread handle.  */
     return ESRCH;
 
@@ -53,15 +56,15 @@ __pthread_kill (threadid, signo)
   int val;
 #if __ASSUME_TGKILL
   val = INTERNAL_SYSCALL (tgkill, err, 3, THREAD_GETMEM (THREAD_SELF, pid),
-			  pd->tid, signo);
+			  tid, signo);
 #else
 # ifdef __NR_tgkill
   val = INTERNAL_SYSCALL (tgkill, err, 3, THREAD_GETMEM (THREAD_SELF, pid),
-			  pd->tid, signo);
+			  tid, signo);
   if (INTERNAL_SYSCALL_ERROR_P (val, err)
       && INTERNAL_SYSCALL_ERRNO (val, err) == ENOSYS)
 # endif
-    val = INTERNAL_SYSCALL (tkill, err, 2, pd->tid, signo);
+    val = INTERNAL_SYSCALL (tkill, err, 2, tid, signo);
 #endif
 
   return (INTERNAL_SYSCALL_ERROR_P (val, err)


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