From 3c9c61febede148b79d8509e16588152d99b3774 Mon Sep 17 00:00:00 2001 From: Torvald Riegel Date: Tue, 21 Apr 2015 20:34:21 +0200 Subject: [PATCH] Fix lost wake-up when pthread_rwlock_timedrwlock times out. If we set up a rwlock to prefer writers (and disallow recursive rdlock acquisitions), then readers will block for writers that are blocked to acquire the lock (otherwise, readers could constantly enter and exit, and the writer would never get the lock). However, the existing implementation did not wake such readers when the writer timed out. This patch adds the missing wake-up. There's no similar case for writers being blocked on readers. --- ChangeLog | 8 +++ NEWS | 5 +- nptl/Makefile | 1 + nptl/pthread_rwlock_timedwrlock.c | 18 +++++ nptl/tst-rwlock15.c | 116 ++++++++++++++++++++++++++++++ 5 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 nptl/tst-rwlock15.c diff --git a/ChangeLog b/ChangeLog index e0e6d6b210..423ccea936 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2015-06-04 Torvald Riegel + + [BZ #18324] + * nptl/pthread_rwlock_timedwrlock.c (pthread_rwlock_timedwrlock): Add + missing wake-up of readers. + * nptl/tst-rwlock15.c: New file. + * nptl/Makefile (tests): Add new test. + 2015-06-03 Roland McGrath * sysdeps/nacl/nacl-interfaces.c (try_supply): New static function. diff --git a/NEWS b/NEWS index 43c2cc7b54..f8ad875865 100644 --- a/NEWS +++ b/NEWS @@ -18,8 +18,9 @@ Version 2.22 18020, 18029, 18030, 18032, 18036, 18038, 18039, 18042, 18043, 18046, 18047, 18049, 18068, 18080, 18093, 18100, 18104, 18110, 18111, 18116, 18125, 18128, 18138, 18185, 18196, 18197, 18206, 18210, 18211, 18217, - 18220, 18221, 18234, 18244, 18247, 18287, 18319, 18333, 18346, 18397, - 18409, 18410, 18412, 18418, 18422, 18434, 18444, 18468, 18469, 18470. + 18220, 18221, 18234, 18244, 18247, 18287, 18319, 18324, 18333, 18346, + 18397, 18409, 18410, 18412, 18418, 18422, 18434, 18444, 18468, 18469, + 18470. * Cache information can be queried via sysconf() function on s390 e.g. with _SC_LEVEL1_ICACHE_SIZE as argument. diff --git a/nptl/Makefile b/nptl/Makefile index 3dd2944260..62436ac48f 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -231,6 +231,7 @@ tests = tst-typesizes \ tst-rwlock1 tst-rwlock2 tst-rwlock2a tst-rwlock3 tst-rwlock4 \ tst-rwlock5 tst-rwlock6 tst-rwlock7 tst-rwlock8 tst-rwlock9 \ tst-rwlock10 tst-rwlock11 tst-rwlock12 tst-rwlock13 tst-rwlock14 \ + tst-rwlock15 \ tst-once1 tst-once2 tst-once3 tst-once4 \ tst-key1 tst-key2 tst-key3 tst-key4 \ tst-sem1 tst-sem2 tst-sem3 tst-sem4 tst-sem5 tst-sem6 tst-sem7 \ diff --git a/nptl/pthread_rwlock_timedwrlock.c b/nptl/pthread_rwlock_timedwrlock.c index 416f6e2bd5..a759fa997f 100644 --- a/nptl/pthread_rwlock_timedwrlock.c +++ b/nptl/pthread_rwlock_timedwrlock.c @@ -23,6 +23,7 @@ #include #include #include +#include /* Try to acquire write lock for RWLOCK or return after specfied time. */ @@ -32,6 +33,7 @@ pthread_rwlock_timedwrlock (rwlock, abstime) const struct timespec *abstime; { int result = 0; + bool wake_readers = false; /* Make sure we are alone. */ lll_lock (rwlock->__data.__lock, rwlock->__data.__shared); @@ -136,6 +138,17 @@ pthread_rwlock_timedwrlock (rwlock, abstime) if (err == -ETIMEDOUT) { result = ETIMEDOUT; + /* If we prefer writers, it can have happened that readers blocked + for us to acquire the lock first. If we have timed out, we need + to wake such readers if there are any, and if there is no writer + currently (otherwise, the writer will take care of wake-up). */ + if (!PTHREAD_RWLOCK_PREFER_READER_P (rwlock) + && (rwlock->__data.__nr_readers_queued > 0) + && (rwlock->__data.__writer == 0)) + { + ++rwlock->__data.__readers_wakeup; + wake_readers = true; + } break; } } @@ -143,5 +156,10 @@ pthread_rwlock_timedwrlock (rwlock, abstime) /* We are done, free the lock. */ lll_unlock (rwlock->__data.__lock, rwlock->__data.__shared); + /* Might be required after timeouts. */ + if (wake_readers) + lll_futex_wake (&rwlock->__data.__readers_wakeup, INT_MAX, + rwlock->__data.__shared); + return result; } diff --git a/nptl/tst-rwlock15.c b/nptl/tst-rwlock15.c new file mode 100644 index 0000000000..b292701e21 --- /dev/null +++ b/nptl/tst-rwlock15.c @@ -0,0 +1,116 @@ +/* Copyright (C) 2015 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 + . */ + +/* This tests that a writer that is preferred -- but times out due to a + reader being present -- does not miss to wake other readers blocked on the + writer's pending lock acquisition. */ + +#include +#include +#include +#include +#include + +/* The bug existed in the code that strictly prefers writers over readers. */ +static pthread_rwlock_t r = PTHREAD_RWLOCK_WRITER_NONRECURSIVE_INITIALIZER_NP; + +static void * +writer (void *arg) +{ + struct timespec ts; + if (clock_gettime (CLOCK_REALTIME, &ts) != 0) + { + puts ("clock_gettime failed"); + exit (EXIT_FAILURE); + } + ts.tv_sec += 1; + int e = pthread_rwlock_timedwrlock (&r, &ts); + if (e != ETIMEDOUT) + { + puts ("timedwrlock did not time out"); + exit (EXIT_FAILURE); + } + return NULL; +} + +static void * +reader (void *arg) +{ + /* This isn't a reliable way to get the interleaving we need (because a + failed trylock doesn't synchronize with the writer, and because we could + try to lock after the writer has already timed out). However, both will + just lead to false positives. */ + int e; + while ((e = pthread_rwlock_tryrdlock (&r)) != EBUSY) + { + if (e != 0) + exit (EXIT_FAILURE); + pthread_rwlock_unlock (&r); + } + e = pthread_rwlock_rdlock (&r); + if (e != 0) + { + puts ("reader rdlock failed"); + exit (EXIT_FAILURE); + } + pthread_rwlock_unlock (&r); + return NULL; +} + + +static int +do_test (void) +{ + /* Grab a rdlock, then create a writer and a reader, and wait until they + finished. */ + + if (pthread_rwlock_rdlock (&r) != 0) + { + puts ("initial rdlock failed"); + return 1; + } + + pthread_t thw; + if (pthread_create (&thw, NULL, writer, NULL) != 0) + { + puts ("create failed"); + return 1; + } + pthread_t thr; + if (pthread_create (&thr, NULL, reader, NULL) != 0) + { + puts ("create failed"); + return 1; + } + + if (pthread_join (thw, NULL) != 0) + { + puts ("writer join failed"); + return 1; + } + if (pthread_join (thr, NULL) != 0) + { + puts ("reader join failed"); + return 1; + } + + return 0; +} + + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" -- 2.43.5