timer_create () abort

[Daniel Jacobowitz <drow at mvista dot com>]

If you try to run this testcase (at least with LinuxThreads...):

<cut>
#include <stdio.h>
#include <time.h>
#include <signal.h>
#include <pthread.h>
#include <sys/time.h>
#include <sys/resource.h>
#include <unistd.h>

void
thread (union sigval arg)
{
  printf ("Timeout.\n");
}

int
main (void)
{
  int i, res;
  timer_t timerId;
  struct itimerspec itval;
  struct sigevent sigev;

  itval.it_interval.tv_sec = 2;
  itval.it_interval.tv_nsec = 0;
  itval.it_value.tv_sec = 2;
  itval.it_value.tv_nsec = 0;

  sigev.sigev_notify = SIGEV_THREAD;
  sigev.sigev_signo = SIGRTMIN;
  sigev.sigev_notify_function = thread;
  sigev.sigev_notify_attributes = 0;
  sigev.sigev_value.sival_ptr = (void *) &timerId;

  for (i = 0; i < 33; i++)
    {
      printf ("cnt = %d\n", i);

      if (timer_create (CLOCK_REALTIME, &sigev, &timerId) < 0)
	perror ("timer_create");

      res = timer_settime (timerId, 0, &itval, NULL);
      if (res < 0)
	perror ("timer_settime");

      res = timer_delete (timerId);
      if (res < 0)
	perror ("timer_delete");
    }

  return 0;
}
</cut>

then you'll see two wrong things happen.  For the last timer, we fail to
create a timer, and then we hit an assertion:

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?


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.  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.  It limits the
number of possible (attr, clock) pairs an application can use but that's no
loss.

I don't see the purpose of the list_unlink in __timer_thread_find_matching,
and removing it fixes the problem.  Without it no thread's queue is ever
more than one timer deep.  The unlink's been there as long as the file's
been in CVS; my best guess is that someone was testing the multiple-threads
part of the timer implementation and never took it out.  This patch passes
make check on i686-linux; is it OK?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-06-05  Daniel Jacobowitz  <drow@mvista.com>

	* sysdeps/pthread/timer_routines.c (__timer_thread_find_matching):
	Remove list_unlink call.

--- linuxthreads/sysdeps/pthread/timer_routines.c.orig	2003-06-05 17:44:39.000000000 -0400
+++ linuxthreads/sysdeps/pthread/timer_routines.c	2003-06-05 17:41:25.000000000 -0400
@@ -539,7 +539,6 @@ __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;
         }