* Bradley T. Hughes <[EMAIL PROTECTED]> [000812 09:16] wrote:
> It seems that the mutex implementation for recursive mutexes has a bug.
> 
> a single lock/unlock pair works as expected, but locking 3 times only
> requires 2 unlocks before the mutex is released.  i've read through
> src/lib/libc_r/uthread/uthread_mutex.c but can't quite follow the logic...
> 
> i plan on debugging this more and hope to generate a patch... but i
> figured i'd send off an email about it to see if anyone else can find the
> solution quicker than i can :)

It looks like an off-by-one-error in libc_r, let me know if this works:

Index: uthread_mutex.c
===================================================================
RCS file: /home/ncvs/src/lib/libc_r/uthread/uthread_mutex.c,v
retrieving revision 1.22
diff -u -u -r1.22 uthread_mutex.c
--- uthread_mutex.c     2000/06/14 17:17:41     1.22
+++ uthread_mutex.c     2000/08/12 16:41:47
@@ -753,7 +753,7 @@
                                ret = (*mutex)->m_owner == NULL ? EINVAL : EPERM;
                        }
                        else if (((*mutex)->m_type == PTHREAD_MUTEX_RECURSIVE) &&
-                           ((*mutex)->m_data.m_count > 1)) {
+                           ((*mutex)->m_data.m_count > 0)) {
                                /* Decrement the count: */
                                (*mutex)->m_data.m_count--;
                        } else {
@@ -821,7 +821,7 @@
                                ret = (*mutex)->m_owner == NULL ? EINVAL : EPERM;
                        }
                        else if (((*mutex)->m_type == PTHREAD_MUTEX_RECURSIVE) &&
-                           ((*mutex)->m_data.m_count > 1)) {
+                           ((*mutex)->m_data.m_count > 0)) {
                                /* Decrement the count: */
                                (*mutex)->m_data.m_count--;
                        } else {
@@ -939,7 +939,7 @@
                                ret = (*mutex)->m_owner == NULL ? EINVAL : EPERM;
                        }
                        else if (((*mutex)->m_type == PTHREAD_MUTEX_RECURSIVE) &&
-                           ((*mutex)->m_data.m_count > 1)) {
+                           ((*mutex)->m_data.m_count > 0)) {
                                /* Decrement the count: */
                                (*mutex)->m_data.m_count--;
                        } else {

-Alfred

Attached so Jason and Daniel can have a look:

> 
> to see the bug, take the attached blah.c and compile it:
> 
> gcc -pthread -o blah blah.c
> 
> and run it :
> 
> ./blah
> 
> --
> Bradley T. Hughes <[EMAIL PROTECTED]>
> Waldemar Thranes gt. 98B N-0175 Oslo, Norway
> Office: +47 21 60 48 92
> Mobile: +47 92 01 97 81

Content-Description: blah.c
> #include <pthread.h>
> #include <stdio.h>
> #include <string.h>
> #include <unistd.h>
> 
> pthread_cond_t cond;
> 
> void *thread1(void *arg) {
>     pthread_mutex_t *mutex = (pthread_mutex_t *) arg;
>     
>     // lock/unlock once works fine
>     printf("thread 1 locking mutex (%s)\n",
>            strerror(pthread_mutex_lock(mutex)));
>     
>     // make sure thread1 is first
>     pthread_cond_wait(&cond, mutex);
>     pthread_cond_signal(&cond);
>     
>     printf("thread 1 unlocking mutex (%s)\n",
>            strerror(pthread_mutex_unlock(mutex)));
>     
>     // lock 6 times and then unlock 6 times
>     printf("thread 1 locking mutex 1 (%s)\n",
>            strerror(pthread_mutex_lock(mutex)));
>     printf("thread 1 locking mutex 2 (%s)\n",
>            strerror(pthread_mutex_lock(mutex)));
>     printf("thread 1 locking mutex 3 (%s)\n",
>            strerror(pthread_mutex_lock(mutex)));
>     printf("thread 1 locking mutex 4 (%s)\n",
>            strerror(pthread_mutex_lock(mutex)));
>     printf("thread 1 locking mutex 5 (%s)\n",
>            strerror(pthread_mutex_lock(mutex)));
>     printf("thread 1 locking mutex 6 (%s)\n",
>            strerror(pthread_mutex_lock(mutex)));
> 
>     printf("thread 1 unlocking mutex 6 (%s)\n",
>            strerror(pthread_mutex_unlock(mutex)));
>     printf("thread 1 unlocking mutex 5 (%s)\n",
>            strerror(pthread_mutex_unlock(mutex)));
>     printf("thread 1 unlocking mutex 4 (%s)\n",
>            strerror(pthread_mutex_unlock(mutex)));
>     printf("thread 1 unlocking mutex 3 (%s)\n",
>            strerror(pthread_mutex_unlock(mutex)));
>     printf("thread 1 unlocking mutex 2 (%s)\n",
>            strerror(pthread_mutex_unlock(mutex)));
>     
>     printf("!!! should still be locked, but the other thread will be able to run 
>now\n");
>     sleep(1);
>     printf("thread 1 unlocking mutex 1 (%s)\n",
>            strerror(pthread_mutex_unlock(mutex)));
>     printf("!!! notice the errors\n");
>     
>     return NULL;
> }
> 
> 
> void *thread2(void *arg) {
>     pthread_mutex_t *mutex = (pthread_mutex_t *) arg;
>     
>     printf("thread 2 locking mutex (%s)\n",
>            strerror(pthread_mutex_lock(mutex)));
> 
>     pthread_cond_signal(&cond);
>     pthread_cond_wait(&cond, mutex);
> 
>     printf("thread 2 unlocking mutex (%s)\n",
>            strerror(pthread_mutex_unlock(mutex)));
> 
>     printf("thread 2 locking mutex 1 (%s)\n",
>          strerror(pthread_mutex_lock(mutex)));
>     printf("thread 2 locking mutex 2 (%s)\n",
>            strerror(pthread_mutex_lock(mutex)));
>     printf("thread 2 locking mutex 3 (%s)\n",
>            strerror(pthread_mutex_lock(mutex)));
>     printf("thread 2 locking mutex 4 (%s)\n",
>            strerror(pthread_mutex_lock(mutex)));
>     printf("thread 2 locking mutex 5 (%s)\n",
>            strerror(pthread_mutex_lock(mutex)));
>     printf("thread 2 locking mutex 6 (%s)\n",
>            strerror(pthread_mutex_lock(mutex)));
> 
>     printf("thread 2 unlocking mutex 6 (%s)\n",
>            strerror(pthread_mutex_unlock(mutex)));
>     printf("thread 2 unlocking mutex 5 (%s)\n",
>            strerror(pthread_mutex_unlock(mutex)));
>     printf("thread 2 unlocking mutex 4 (%s)\n",
>            strerror(pthread_mutex_unlock(mutex)));
>     printf("thread 2 unlocking mutex 3 (%s)\n",
>            strerror(pthread_mutex_unlock(mutex)));
>     printf("thread 2 unlocking mutex 2 (%s)\n",
>            strerror(pthread_mutex_unlock(mutex)));
> 
>     // should be locked, but it is not
>     printf("thread 2 unlocking mutex 1 (%s)\n",
>            strerror(pthread_mutex_unlock(mutex)));
> 
>     return NULL;
> }
> 
> 
> int main() {
>     pthread_t thr1, thr2;
>     pthread_mutex_t mutex;
>     void *junk;
> 
>     pthread_mutexattr_t mattr;
>     pthread_mutexattr_init(&mattr);
>     pthread_mutexattr_settype(&mattr, PTHREAD_MUTEX_RECURSIVE);
>     pthread_mutex_init(&mutex, &mattr);
> 
>     pthread_cond_init(&cond, NULL);
> 
>     pthread_create( &thr1, NULL, thread1, (void *) &mutex );
>     pthread_create( &thr2, NULL, thread2, (void *) &mutex );
> 
>     pthread_join(thr1, &junk);
>     pthread_join(thr2, &junk);
> 
>     return 0;
> }


-- 
-Alfred Perlstein - [[EMAIL PROTECTED]|[EMAIL PROTECTED]]
"I have the heart of a child; I keep it in a jar on my desk."


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-stable" in the body of the message

Reply via email to