Date: Sat, 12 Aug 2000 10:33:06 -0700 From: Alfred Perlstein <bright@wintelcom.net> To: "Bradley T. Hughes" <bhughes@trolltech.com> 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> In-Reply-To: <Pine.BSF.4.21.0008121809030.27068-200000@reticent.troll.no>; from bhughes@trolltech.com on Sat, Aug 12, 2000 at 06:14:55PM %2B0200 References: <Pine.BSF.4.21.0008121809030.27068-200000@reticent.troll.no>
next in thread | previous in thread | raw e-mail | index | archive | help
* Bradley T. Hughes <bhughes@trolltech.com> [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 <bhughes@trolltech.com>
> 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 - [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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20000812103306.G4854>