From owner-freebsd-stable Sat Aug 12 10:33:16 2000 Delivered-To: freebsd-stable@freebsd.org Received: from fw.wintelcom.net (ns1.wintelcom.net [209.1.153.20]) by hub.freebsd.org (Postfix) with ESMTP id 180A637BF50; Sat, 12 Aug 2000 10:33:11 -0700 (PDT) (envelope-from bright@fw.wintelcom.net) Received: (from bright@localhost) by fw.wintelcom.net (8.10.0/8.10.0) id e7CHX6x27722; Sat, 12 Aug 2000 10:33:06 -0700 (PDT) Date: Sat, 12 Aug 2000 10:33:06 -0700 From: Alfred Perlstein To: "Bradley T. Hughes" Cc: stable@FreeBSD.ORG, jasone@FreeBSD.ORG, eischen@vigrid.com Subject: (recursive lock problem) Re: bug in pthread implementation Message-ID: <20000812103306.G4854@fw.wintelcom.net> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.4i In-Reply-To: ; from bhughes@trolltech.com on Sat, Aug 12, 2000 at 06:14:55PM +0200 Sender: owner-freebsd-stable@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG * Bradley T. Hughes [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 > 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 > #include > #include > #include > > 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 - [bright@wintelcom.net|alfred@freebsd.org] "I have the heart of a child; I keep it in a jar on my desk." To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-stable" in the body of the message