Take lock in pthread_cond_wait cleanup handler only when needed

[BZ #14652]
When a thread waiting in pthread_cond_wait with a PI mutex is
cancelled after it has returned successfully from the futex syscall
but just before async cancellation is disabled, it enters its
cancellation handler with the mutex held and simply calling a
mutex_lock again will result in a deadlock.  Hence, it is necessary to
see if the thread owns the lock and try to lock it only if it doesn't.
This commit is contained in:
Siddhesh Poyarekar 2012-10-10 12:17:27 +05:30
parent f96f12423a
commit 0e3b5d6a68
9 changed files with 368 additions and 10 deletions

2
NEWS
View File

@ -16,7 +16,7 @@ Version 2.17
14336, 14337, 14347, 14349, 14376, 14417, 14459, 14476, 14477, 14505,
14510, 14516, 14518, 14519, 14530, 14532, 14538, 14543, 14544, 14545,
14557, 14562, 14568, 14576, 14579, 14583, 14587, 14602, 14621, 14638,
14645, 14648, 14660, 14661.
14645, 14648, 14652, 14660, 14661.
* Support for STT_GNU_IFUNC symbols added for s390 and s390x.
Optimized versions of memcpy, memset, and memcmp added for System z10 and

View File

@ -1,3 +1,21 @@
2012-10-10 Siddhesh Poyarekar <siddhesh@redhat.com>
[BZ #14652]
* Makefile (tests): New test case tst-cond25.
(LDFLAGS-tst-cond25): Link tst-cond25 against librt.
* sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S
(__condvar_tw_cleanup): Lock mutex only if we don't already
own it.
* sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S
(__condvar_w_cleanup): Likewise.
* sysdeps/unix/sysv/linux/pthread-pi-defines.sym: Add TID_MASK.
* sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S
(__condvar_cleanup2): Lock mutex only if we don't already
own it.
* sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
(__condvar_cleanup1): Likewise.
* tst-cond25.c: New test case.
2012-10-09 Roland McGrath <roland@hack.frob.com>
* sysdeps/pthread/configure: Regenerated.

View File

@ -206,7 +206,7 @@ tests = tst-typesizes \
tst-cond1 tst-cond2 tst-cond3 tst-cond4 tst-cond5 tst-cond6 tst-cond7 \
tst-cond8 tst-cond9 tst-cond10 tst-cond11 tst-cond12 tst-cond13 \
tst-cond14 tst-cond15 tst-cond16 tst-cond17 tst-cond18 tst-cond19 \
tst-cond20 tst-cond21 tst-cond22 tst-cond23 tst-cond24 \
tst-cond20 tst-cond21 tst-cond22 tst-cond23 tst-cond24 tst-cond25 \
tst-cond-except \
tst-robust1 tst-robust2 tst-robust3 tst-robust4 tst-robust5 \
tst-robust6 tst-robust7 tst-robust8 tst-robust9 \
@ -276,6 +276,7 @@ gen-as-const-headers = pthread-errnos.sym
LDFLAGS-pthread.so = -Wl,--enable-new-dtags,-z,nodelete,-z,initfirst
LDFLAGS-tst-cond24 = -lrt
LDFLAGS-tst-cond25 = -lrt
include ../Makeconfig

View File

@ -649,10 +649,24 @@ __condvar_tw_cleanup:
movl $0x7fffffff, %edx
ENTER_KERNEL
/* Lock the mutex only if we don't own it already. This only happens
in case of PI mutexes, if we got cancelled after a successful
return of the futex syscall and before disabling async
cancellation. */
5: movl 24+FRAME_SIZE(%esp), %eax
call __pthread_mutex_cond_lock
movl MUTEX_KIND(%eax), %ebx
andl $(ROBUST_BIT|PI_BIT), %ebx
cmpl $PI_BIT, %ebx
jne 8f
movl %esi, (%esp)
movl (%eax), %ebx
andl $TID_MASK, %ebx
cmpl %ebx, %gs:TID
je 9f
8: call __pthread_mutex_cond_lock
9: movl %esi, (%esp)
.LcallUR:
call _Unwind_Resume
hlt

View File

@ -566,10 +566,24 @@ __condvar_w_cleanup:
movl $0x7fffffff, %edx
ENTER_KERNEL
/* Lock the mutex only if we don't own it already. This only happens
in case of PI mutexes, if we got cancelled after a successful
return of the futex syscall and before disabling async
cancellation. */
5: movl 24+FRAME_SIZE(%esp), %eax
call __pthread_mutex_cond_lock
movl MUTEX_KIND(%eax), %ebx
andl $(ROBUST_BIT|PI_BIT), %ebx
cmpl $PI_BIT, %ebx
jne 8f
movl %esi, (%esp)
movl (%eax), %ebx
andl $TID_MASK, %ebx
cmpl %ebx, %gs:TID
je 9f
8: call __pthread_mutex_cond_lock
9: movl %esi, (%esp)
.LcallUR:
call _Unwind_Resume
hlt

View File

@ -6,3 +6,4 @@ MUTEX_KIND offsetof (pthread_mutex_t, __data.__kind)
ROBUST_BIT PTHREAD_MUTEX_ROBUST_NORMAL_NP
PI_BIT PTHREAD_MUTEX_PRIO_INHERIT_NP
PS_BIT PTHREAD_MUTEX_PSHARED_BIT
TID_MASK FUTEX_TID_MASK

View File

@ -771,10 +771,24 @@ __condvar_cleanup2:
movl $SYS_futex, %eax
syscall
/* Lock the mutex only if we don't own it already. This only happens
in case of PI mutexes, if we got cancelled after a successful
return of the futex syscall and before disabling async
cancellation. */
5: movq 16(%rsp), %rdi
callq __pthread_mutex_cond_lock
movl MUTEX_KIND(%rdi), %eax
andl $(ROBUST_BIT|PI_BIT), %eax
cmpl $PI_BIT, %eax
jne 7f
movq 24(%rsp), %rdi
movl (%rdi), %eax
andl $TID_MASK, %eax
cmpl %eax, %fs:TID
je 8f
7: callq __pthread_mutex_cond_lock
8: movq 24(%rsp), %rdi
movq FRAME_SIZE(%rsp), %r15
movq FRAME_SIZE+8(%rsp), %r14
movq FRAME_SIZE+16(%rsp), %r13

View File

@ -495,10 +495,24 @@ __condvar_cleanup1:
movl $SYS_futex, %eax
syscall
/* Lock the mutex only if we don't own it already. This only happens
in case of PI mutexes, if we got cancelled after a successful
return of the futex syscall and before disabling async
cancellation. */
5: movq 16(%rsp), %rdi
callq __pthread_mutex_cond_lock
movl MUTEX_KIND(%rdi), %eax
andl $(ROBUST_BIT|PI_BIT), %eax
cmpl $PI_BIT, %eax
jne 7f
movq 24(%rsp), %rdi
movl (%rdi), %eax
andl $TID_MASK, %eax
cmpl %eax, %fs:TID
je 8f
7: callq __pthread_mutex_cond_lock
8: movq 24(%rsp), %rdi
.LcallUR:
call _Unwind_Resume@PLT
hlt

282
nptl/tst-cond25.c Normal file
View File

@ -0,0 +1,282 @@
/* Verify that condition variables synchronized by PI mutexes don't hang on
on cancellation.
Copyright (C) 2012 Free Software Foundation, Inc.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public
License as published by the Free Software Foundation; either
version 2.1 of the License, or (at your option) any later version.
The GNU C Library is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
Lesser General Public License for more details.
You should have received a copy of the GNU Lesser General Public
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/syscall.h>
#include <unistd.h>
#include <sys/time.h>
#include <time.h>
#define NUM 5
#define ITERS 10000
#define COUNT 100
typedef void *(*thr_func) (void *);
pthread_mutex_t mutex;
pthread_cond_t cond;
void cleanup (void *u)
{
/* pthread_cond_wait should always return with the mutex locked. */
if (pthread_mutex_unlock (&mutex))
abort ();
}
void *
signaller (void *u)
{
int i, ret = 0;
void *tret = NULL;
for (i = 0; i < ITERS; i++)
{
if ((ret = pthread_mutex_lock (&mutex)) != 0)
{
tret = (void *)1;
printf ("signaller:mutex_lock failed: %s\n", strerror (ret));
goto out;
}
if ((ret = pthread_cond_signal (&cond)) != 0)
{
tret = (void *)1;
printf ("signaller:signal failed: %s\n", strerror (ret));
goto unlock_out;
}
if ((ret = pthread_mutex_unlock (&mutex)) != 0)
{
tret = (void *)1;
printf ("signaller:mutex_unlock failed: %s\n", strerror (ret));
goto out;
}
pthread_testcancel ();
}
out:
return tret;
unlock_out:
if ((ret = pthread_mutex_unlock (&mutex)) != 0)
printf ("signaller:mutex_unlock[2] failed: %s\n", strerror (ret));
goto out;
}
void *
waiter (void *u)
{
int i, ret = 0;
void *tret = NULL;
int seq = (int)u;
for (i = 0; i < ITERS / NUM; i++)
{
if ((ret = pthread_mutex_lock (&mutex)) != 0)
{
tret = (void *)1;
printf ("waiter[%u]:mutex_lock failed: %s\n", seq, strerror (ret));
goto out;
}
pthread_cleanup_push (cleanup, NULL);
if ((ret = pthread_cond_wait (&cond, &mutex)) != 0)
{
tret = (void *)1;
printf ("waiter[%u]:wait failed: %s\n", seq, strerror (ret));
goto unlock_out;
}
if ((ret = pthread_mutex_unlock (&mutex)) != 0)
{
tret = (void *)1;
printf ("waiter[%u]:mutex_unlock failed: %s\n", seq, strerror (ret));
goto out;
}
pthread_cleanup_pop (0);
}
out:
puts ("waiter tests done");
return tret;
unlock_out:
if ((ret = pthread_mutex_unlock (&mutex)) != 0)
printf ("waiter:mutex_unlock[2] failed: %s\n", strerror (ret));
goto out;
}
void *
timed_waiter (void *u)
{
int i, ret;
void *tret = NULL;
int seq = (int)u;
for (i = 0; i < ITERS / NUM; i++)
{
struct timespec ts;
if ((ret = clock_gettime(CLOCK_REALTIME, &ts)) != 0)
{
tret = (void *)1;
printf ("%u:clock_gettime failed: %s\n", seq, strerror (errno));
goto out;
}
ts.tv_sec += 20;
if ((ret = pthread_mutex_lock (&mutex)) != 0)
{
tret = (void *)1;
printf ("waiter[%u]:mutex_lock failed: %s\n", seq, strerror (ret));
goto out;
}
pthread_cleanup_push (cleanup, NULL);
/* We should not time out either. */
if ((ret = pthread_cond_timedwait (&cond, &mutex, &ts)) != 0)
{
tret = (void *)1;
printf ("waiter[%u]:timedwait failed: %s\n", seq, strerror (ret));
goto unlock_out;
}
if ((ret = pthread_mutex_unlock (&mutex)) != 0)
{
tret = (void *)1;
printf ("waiter[%u]:mutex_unlock failed: %s\n", seq, strerror (ret));
goto out;
}
pthread_cleanup_pop (0);
}
out:
puts ("timed_waiter tests done");
return tret;
unlock_out:
if ((ret = pthread_mutex_unlock (&mutex)) != 0)
printf ("waiter[%u]:mutex_unlock[2] failed: %s\n", seq, strerror (ret));
goto out;
}
int
do_test_wait (thr_func f)
{
pthread_t w[NUM];
pthread_t s;
pthread_mutexattr_t attr;
int i, j, ret = 0;
void *thr_ret;
for (i = 0; i < COUNT; i++)
{
if ((ret = pthread_mutexattr_init (&attr)) != 0)
{
printf ("mutexattr_init failed: %s\n", strerror (ret));
goto out;
}
if ((ret = pthread_mutexattr_setprotocol (&attr, PTHREAD_PRIO_INHERIT)) != 0)
{
printf ("mutexattr_setprotocol failed: %s\n", strerror (ret));
goto out;
}
if ((ret = pthread_cond_init (&cond, NULL)) != 0)
{
printf ("cond_init failed: %s\n", strerror (ret));
goto out;
}
if ((ret = pthread_mutex_init (&mutex, &attr)) != 0)
{
printf ("mutex_init failed: %s\n", strerror (ret));
goto out;
}
for (j = 0; j < NUM; j++)
if ((ret = pthread_create (&w[j], NULL, f, (void *)j)) != 0)
{
printf ("waiter[%d]: create failed: %s\n", j, strerror (ret));
goto out;
}
if ((ret = pthread_create (&s, NULL, signaller, NULL)) != 0)
{
printf ("signaller: create failed: %s\n", strerror (ret));
goto out;
}
for (j = 0; j < NUM; j++)
{
if ((ret = pthread_cancel (w[j])) != 0)
{
printf ("waiter[%d]: cancel failed: %s\n", j, strerror (ret));
goto out;
}
if ((ret = pthread_join (w[j], &thr_ret)) != 0)
{
printf ("waiter[%d]: join failed: %s\n", j, strerror (ret));
goto out;
}
if (thr_ret != NULL && thr_ret != PTHREAD_CANCELED)
{
ret = 1;
goto out;
}
}
/* The signalling thread could have ended before it was cancelled. */
pthread_cancel (s);
if ((ret = pthread_join (s, &thr_ret)) != 0)
{
printf ("signaller: join failed: %s\n", strerror (ret));
goto out;
}
if (thr_ret != NULL && thr_ret != PTHREAD_CANCELED)
{
ret = 1;
goto out;
}
}
out:
return ret;
}
int
do_test (int argc, char **argv)
{
int ret = do_test_wait (waiter);
if (ret)
return ret;
return do_test_wait (timed_waiter);
}
#define TIMEOUT 5
#include "../test-skeleton.c"