This is the mail archive of the
libc-alpha@sources.redhat.com
mailing list for the glibc project.
Re: timer_create () abort
On Thu, Jun 05, 2003 at 03:50:32PM -0700, Kaz Kylheku wrote:
> On Thu, 5 Jun 2003, Daniel Jacobowitz wrote:
>
> > timer: ../linuxthreads/sysdeps/pthread/timer_routines.c:578: __timer_dealloc: Assertion `timer->refcount == 0' failed.
>
> > The assertion's easy: __timer_alloc sets the count to 1, __timer_dealloc
> > assumes it will be 0, we never decrease the refcount before deallocating it
> > on the EAGAIN exit path. I think just adding a timer_delref before the call
> > to __timer_dealloc is the right fix. Thoughts?
>
> That is correct. The decrementing of the refcount and deletion should
> probably have been combined into one operation.
>
> > But why are we failing at all? Well, we're out of threads.
> > __timer_thread_alloc looks on the thread free list for something to move to
> > the active list; __timer_dealloc moves it from the active list to the free
> > list; but __timer_thread_find_matching removes from the active list and it
> > doesn't get put back.
>
> This looks like a bug. This function should quite simply not be
> removing the thread from the active list. To be on the active list
> simply means to be an allocated valid thread; it's just a way of
> keeping track of the allocated thread so that this function can do its
> job of finding a matching one.
>
> This is nothing but good old object interning at work. We want two
> threads that have the same set of properties to be the same object, so
> that for any unique set of thread properties (priority, etc) there is
> just one thread handling all the timers which want that type of thread.
>
> > So once a thread gets one timer, it never gets
> > another. And when the timer is deleted, the thread does not exit, so we
> > never have room for a new thread - I think that's by design.
>
> This is a straight leak; effectively, be removing the thread from the
> active list, we are losing a reference without destroying it.
>
> When a timer is deleted, we don't want the thread to exit, because it
> can have other timers. Essentially, threads persist. There could be
> some additional logic in timer_delete to kill the thread and delete the
> thread structure if this is the last timer being serviced by that
> thread.
>
> The design assumes that the appliaction will only generate a small
> variety of different kinds of threads for SIGEV_THREAD notification. So
> there will just be at most a tiny pool of threads in the active list,
> and these can just endure over the lifetime of the application.
Thanks. In that case, would a glibc maintainer please consider this
patch?
--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer
2003-06-19 Daniel Jacobowitz <drow@mvista.com>
* sysdeps/pthread/timer_create.c (timer_create): Call timer_delref
before __timer_dealloc.
* sysdeps/pthread/timer_routines.c (__timer_thread_find_matching):
Don't call list_unlink.
Index: linuxthreads/sysdeps/pthread/timer_create.c
===================================================================
RCS file: /cvs/glibc/libc/linuxthreads/sysdeps/pthread/timer_create.c,v
retrieving revision 1.8
diff -u -p -r1.8 timer_create.c
--- linuxthreads/sysdeps/pthread/timer_create.c 3 Mar 2003 05:28:17 -0000 1.8
+++ linuxthreads/sysdeps/pthread/timer_create.c 20 Jun 2003 03:24:50 -0000
@@ -178,7 +178,10 @@ timer_create (clock_id, evp, timerid)
if (thread != NULL)
__timer_thread_dealloc (thread);
if (newtimer != NULL)
- __timer_dealloc (newtimer);
+ {
+ timer_delref (newtimer);
+ __timer_dealloc (newtimer);
+ }
}
pthread_mutex_unlock (&__timer_mutex);
Index: linuxthreads/sysdeps/pthread/timer_routines.c
===================================================================
RCS file: /cvs/glibc/libc/linuxthreads/sysdeps/pthread/timer_routines.c,v
retrieving revision 1.21
diff -u -p -r1.21 timer_routines.c
--- linuxthreads/sysdeps/pthread/timer_routines.c 29 Aug 2002 01:41:17 -0000 1.21
+++ linuxthreads/sysdeps/pthread/timer_routines.c 20 Jun 2003 03:24:51 -0000
@@ -538,10 +538,7 @@ __timer_thread_find_matching (const pthr
if (thread_attr_compare (desired_attr, &candidate->attr)
&& desired_clock_id == candidate->clock_id)
- {
- list_unlink (iter);
- return candidate;
- }
+ return candidate;
iter = list_next (iter);
}