Fwd: problems with linuxthreads under your latest glibc 15C rpms

[Franz Sirl <Franz dot Sirl-kernel at lauterbach dot com>]

Hi,

this is a heads-up notice for distribution maintainers using the 
glibc-2-1-branch. It seems the branch contains a threads bug introduced with:

2000-03-16  Ulrich Drepper  <drepper@redhat.com>

         * mutex.c (__pthread_mutex_lock): Always initialize __m_owner.
         (__pthread_mutex_trylock): Likewise.
         (__pthread_mutex_unlock): Always clear __m_owner.

Check the appended message for details. I've added the patch to my source 
tree, maybe someone finds time to check it in into the branch (yes, I know 
that the branch is more or less dead :-) ).

Thanks,
Franz.


>From: "Kevin B. Hendricks" <khendricks@ivey.uwo.ca>
>To: Franz Sirl <Franz.Sirl-kernel@lauterbach.com>,
>         linuxppc-dev@lists.linuxppc.org
>Subject: problems with linuxthreads under your latest glibc 15C rpms
>
>I am comparing your src rpms for linuxthreads and noticed that your glibc 
>2.1.3
>-15c has a bug in its implementation of pthread_mutex_trylock.
>
>I don't think anyone *ever* tests the native threads changes made to glibc!
>
>Here is the code snippet:
>
>int __pthread_mutex_trylock(pthread_mutex_t * mutex)
>{
>   pthread_descr self;
>   int retcode;
>
>   switch(mutex->__m_kind) {
>   case PTHREAD_MUTEX_FAST_NP:
>     retcode = __pthread_trylock(&mutex->__m_lock);
>     mutex->__m_owner = thread_self();
>     return retcode;
>   case PTHREAD_MUTEX_RECURSIVE_NP:
>     self = thread_self();
>     if (mutex->__m_owner == self) {
>       mutex->__m_count++;
>       return 0;
>     }
>     retcode = __pthread_trylock(&mutex->__m_lock);
>     if (retcode == 0) {
>       mutex->__m_owner = self;
>       mutex->__m_count = 0;
>     }
>     return retcode;
>   case PTHREAD_MUTEX_ERRORCHECK_NP:
>     retcode = __pthread_trylock(&mutex->__m_lock);
>     if (retcode == 0) {
>       mutex->__m_owner = thread_self();
>     }
>     return retcode;
>   default:
>     return EINVAL;
>   }
>}
>
>
>Notice that under the typical mutex case PTHREAD_MUTEX_FAST_NP, the call to
>__pthread_trylock is made and the owner is set to be the current thread 
>without
>even checking the retcode.  If someone else owns the lock, __pthread_trylock
>will return EBUSY and the code shound not change the owner of the lock!!!!!
>
>
>In fact, the only real differences in linuxthreads code between glibc-2.1.3-5a
>and glibc-2.1.3-15c  are to keep track of the owner under the FAST_NP lock.
>
>All of these changes should be removed. Unless, someone has changed the spec,
>the lock owner should only be kept track of when using the ERRORCHECK_NP mutex
>type.
>
>In fact, that is exactly what the ERRORCHECK mutex type is for.  If you 
>compare
>the code from the FAST_NP case and the ERRORCHECK_NP case, you will see the
>ERRORCHECK case tests the retcode before setting the owner.
>
>I have not had a chance to check the remainder of the changes in linuxthreads,
>but this one screamed at me when I saw it.
>
>Perhaps you could remove the 15c rpms before they get out there too much.
>
>I will check for other mistakes in the changes and send you a patch to fix any
>other mistakes I find.
>
>--- linuxthreads/mutex.c.prev   Thu Jul 27 20:57:28 2000
>+++ linuxthreads/mutex.c        Thu Jul 27 20:59:06 2000
>@@ -50,7 +50,9 @@
>    switch(mutex->__m_kind) {
>    case PTHREAD_MUTEX_FAST_NP:
>      retcode = __pthread_trylock(&mutex->__m_lock);
>-    mutex->__m_owner = thread_self();
>+    if (retcode == 0) {
>+      mutex->__m_owner = thread_self();
>+    }
>      return retcode;
>    case PTHREAD_MUTEX_RECURSIVE_NP:
>      self = thread_self();
>
>
>I will let you know if I find any more funny things.
>
>Thanks,
>
>Kevin