sem_post() does not signal waiting threads

Hello!

There is an error in the POSIX semaphore implementation in NGPT:

sem_post() will wake up other threads waiting in sem_wait() only,
if the semaphore value was 0 and sem_post() increases the value to 1
only after the wakeup!

The manual page
>      sem_post  atomically  increases the count of the semaphore
>      pointed to by sem.  This function  never  blocks  and  can
>      safely be used in asynchronous signal handlers.
does not say anything about this behaviour. It should always first increase
the semaphore and then wake up other threads (maybe check (or better assert),
if it is now != 0).

The attached patch file corrects this strange behaviour.

The order (increase and signal) is not really important, because the lock
&sem->__sem_waiting->__lock is held during the whole operation.
So an alternative minimal change would be to remove the line
if (sem->__sem_value == 0)
in the file semapore.c/__new_sem_post()

You can see the problem, if you create many threads in a loop and every
thread waits in the same sem_wait. Then sleep for a second to be sure, that all
threads are really waiting. Then call sem_post for as many threads as you
created before and voila: the first thread will wake up but all others will
continue to wait in sem_wait, because the scheduler did not start the first
thread, who could decrease the semaphore to 0 between two calls to sem_post.

So we get the situation of a non-zero semaphore but waiting threads nontheless.

Bye, Uwe

-- 
Dr. Uwe Girlich              email: [email protected]
Philosys Software GmbH       www: www.philosys.de
Edisonstrasse 6              phone: +49 89 321407-44
D-85716 Unterschleissheim    fax: +49 89 321407-12

*** semaphore.c-orig	Sat Jun  1 20:20:57 2002
--- semaphore.c	Fri Aug 30 09:01:52 2002
***************
*** 119,129 ****
  int __new_sem_post(sem_t * sem)
  {
      pthread_mutex_lock(&sem->__sem_waiting->__lock);
-     if (sem->__sem_value == 0)
- 	pthread_cond_signal(&sem->__sem_waiting->nonzero);
      pth_acquire_lock(&sem->__sem_waiting->__sem_lock);
      sem->__sem_value++;
      pth_release_lock(&sem->__sem_waiting->__sem_lock);
      pthread_mutex_unlock(&sem->__sem_waiting->__lock);
      return 0;
  }
--- 119,129 ----
  int __new_sem_post(sem_t * sem)
  {
      pthread_mutex_lock(&sem->__sem_waiting->__lock);
      pth_acquire_lock(&sem->__sem_waiting->__sem_lock);
      sem->__sem_value++;
      pth_release_lock(&sem->__sem_waiting->__sem_lock);
+     if (sem->__sem_value != 0)
+         pthread_cond_signal(&sem->__sem_waiting->nonzero);
      pthread_mutex_unlock(&sem->__sem_waiting->__lock);
      return 0;
  }