This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH][BZ #14652] Adjust mutex lock in condvar_cleanup if we gotit from requeue_pi
- From: Siddhesh Poyarekar <siddhesh at redhat dot com>
- To: libc-alpha at sourceware dot org
- Date: Fri, 12 Oct 2012 17:47:05 +0530
- Subject: [PATCH][BZ #14652] Adjust mutex lock in condvar_cleanup if we gotit from requeue_pi
Hi,
I had committed a fix for BZ #14652, which I now realize is
incomplete. This patch completes the fix. Details below:
A process with waiters and signallers on a condvar with a PI mutex
could hang in a deadlock when a waiter is cancelled just as it returns
from its futex call with the PI mutex held. The cancellation cleanup
handler tries to lock the mutex again and hence ends in a deadlock. My
earlier patch checked whether we already own the mutex and only call
the lock if needed.
The lock acquired by the FUTEX_WAIT_REQUEUE_PI however only sets the
owning TID in the lock value (mutex->__data.__lock) and as such, the
rest of the mutex data structure is inconsistent. The regular flow of
pthread_cond_*wait calls pthread_mutex_cond_lock_adjust to make the
data structure consistent once again. The attached patch does exactly
this.
Not having this fix results in failure of the unlock operation using
pthread_mutex_unlock with EPERM since the __user field of the mutex is
not set. This failure can theoretically be seen with tst-cond25, but
it is extremely rare. Adding PTHREAD_MUTEX_RECURSIVE to the mix
increases the chances somewhat, but not by much.
Tested on x86_64 and i686. OK to commit?
Regards,
Siddhesh
nptl/ChangeLog:
[BZ #14652]
* sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S
(__condvar_tw_cleanup): Adjust the mutex data structure if it
was locked by FUTEX_WAIT_REQUEUE_PI.
* sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait
(__condvar_w_cleanup): Likewise.
* sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
(__condvar_cleanup2): Likewise.
* sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
(__condvar_cleanup1): Likewise.
diff --git a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S
index 884987c..6011f69 100644
--- a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S
+++ b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S
@@ -662,7 +662,10 @@ __condvar_tw_cleanup:
movl (%eax), %ebx
andl $TID_MASK, %ebx
cmpl %ebx, %gs:TID
- je 9f
+ jne 8f
+ /* We managed to get the lock. Fix it up before returning. */
+ call __pthread_mutex_cond_lock_adjust
+ jmp 9f
8: call __pthread_mutex_cond_lock
diff --git a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S
index bf1e5fe..b418be3 100644
--- a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S
+++ b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S
@@ -579,7 +579,10 @@ __condvar_w_cleanup:
movl (%eax), %ebx
andl $TID_MASK, %ebx
cmpl %ebx, %gs:TID
- je 9f
+ jne 8f
+ /* We managed to get the lock. Fix it up before returning. */
+ call __pthread_mutex_cond_lock_adjust
+ jmp 9f
8: call __pthread_mutex_cond_lock
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
index eb13326..15e451a 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
@@ -784,7 +784,10 @@ __condvar_cleanup2:
movl (%rdi), %eax
andl $TID_MASK, %eax
cmpl %eax, %fs:TID
- je 8f
+ jne 7f
+ /* We managed to get the lock. Fix it up before returning. */
+ callq __pthread_mutex_cond_lock_adjust
+ jmp 8f
7: callq __pthread_mutex_cond_lock
diff --git a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
index 6c6dc0e..2c6b515 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
+++ b/nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
@@ -508,7 +508,11 @@ __condvar_cleanup1:
movl (%rdi), %eax
andl $TID_MASK, %eax
cmpl %eax, %fs:TID
- je 8f
+ jne 7f
+ /* We managed to get the lock. Fix it up before returning. */
+ callq __pthread_mutex_cond_lock_adjust
+ jmp 8f
+
7: callq __pthread_mutex_cond_lock