From: Bruno Haible Date: Wed, 7 Aug 2024 17:53:37 +0000 (+0200) Subject: pthread-rwlock: Fix default wait queue behaviour on glibc/Linux. X-Git-Url: https://gitweb.git.savannah.gnu.org/gitweb/?a=commitdiff_plain;h=c11354b1e06bfe913d004a6375ca3b82bdd1eeb0;p=gnulib.git pthread-rwlock: Fix default wait queue behaviour on glibc/Linux. * lib/pthread.in.h: If REPLACE_PTHREAD_RWLOCK_INIT is 1 but REPLACE_PTHREAD_RWLOCK_DESTROY is 0, override PTHREAD_RWLOCK_INITIALIZER. * lib/pthread-rwlock.c (pthread_rwlockattr_init) [PTHREAD_RWLOCK_BAD_WAITQUEUE]: New function. (pthread_rwlock_init) [PTHREAD_RWLOCK_BAD_WAITQUEUE]: New function. * m4/pthread-rwlock.m4 (gl_PTHREAD_RWLOCK): Check for reasonable pthread_rwlock wait queue handling. Set REPLACE_PTHREAD_RWLOCK_INIT and REPLACE_PTHREAD_RWLOCKATTR_INIT and define PTHREAD_RWLOCK_BAD_WAITQUEUE if not. * modules/pthread-rwlock (configure.ac): Update GL_COND_OBJ_PTHREAD_RWLOCK condition. * doc/posix-functions/pthread_rwlock_rdlock.texi: Mark the glibc problem as fixed. * doc/posix-functions/pthread_rwlock_tryrdlock.texi: Likewise. * doc/posix-functions/pthread_rwlock_timedrdlock.texi: Likewise. --- diff --git a/ChangeLog b/ChangeLog index 304a8a2d2c..7437e79e70 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,23 @@ +2024-08-07 Bruno Haible + + pthread-rwlock: Fix default wait queue behaviour on glibc/Linux. + * lib/pthread.in.h: If REPLACE_PTHREAD_RWLOCK_INIT is 1 but + REPLACE_PTHREAD_RWLOCK_DESTROY is 0, override + PTHREAD_RWLOCK_INITIALIZER. + * lib/pthread-rwlock.c + (pthread_rwlockattr_init) [PTHREAD_RWLOCK_BAD_WAITQUEUE]: New function. + (pthread_rwlock_init) [PTHREAD_RWLOCK_BAD_WAITQUEUE]: New function. + * m4/pthread-rwlock.m4 (gl_PTHREAD_RWLOCK): Check for reasonable + pthread_rwlock wait queue handling. Set REPLACE_PTHREAD_RWLOCK_INIT and + REPLACE_PTHREAD_RWLOCKATTR_INIT and define PTHREAD_RWLOCK_BAD_WAITQUEUE + if not. + * modules/pthread-rwlock (configure.ac): Update + GL_COND_OBJ_PTHREAD_RWLOCK condition. + * doc/posix-functions/pthread_rwlock_rdlock.texi: Mark the glibc problem + as fixed. + * doc/posix-functions/pthread_rwlock_tryrdlock.texi: Likewise. + * doc/posix-functions/pthread_rwlock_timedrdlock.texi: Likewise. + 2024-08-07 Bruno Haible fchmodat: Fix cross-compilation guess. diff --git a/doc/posix-functions/pthread_rwlock_rdlock.texi b/doc/posix-functions/pthread_rwlock_rdlock.texi index 20620de4bc..36e6d4c097 100644 --- a/doc/posix-functions/pthread_rwlock_rdlock.texi +++ b/doc/posix-functions/pthread_rwlock_rdlock.texi @@ -13,17 +13,15 @@ This function is missing on some platforms: Minix 3.1.8, mingw, MSVC 14, Android 4.3. But the provided replacement is just a dummy on some of these platforms: Minix 3.1.8. -@end itemize - -Portability problems not fixed by Gnulib: -@itemize @item This function prefers readers to writers (meaning, when this function is called on an rwlock that is already taken by one or more readers, and another writer is already waiting to take it, this function may return successfully immediately) -- a behaviour that may lead to writer starvation -- on some platforms: -glibc 2.28. -As a workaround, you can use the @code{gl_rwlock_t} type from the Gnulib -module @samp{lock}. +glibc 2.40. +@end itemize + +Portability problems not fixed by Gnulib: +@itemize @end itemize diff --git a/doc/posix-functions/pthread_rwlock_timedrdlock.texi b/doc/posix-functions/pthread_rwlock_timedrdlock.texi index 614b036553..ba72389cb9 100644 --- a/doc/posix-functions/pthread_rwlock_timedrdlock.texi +++ b/doc/posix-functions/pthread_rwlock_timedrdlock.texi @@ -13,15 +13,15 @@ This function is missing on some platforms: macOS 14, FreeBSD 5.2.1, OpenBSD 3.8, Minix 3.1.8, AIX 5.1, HP-UX 11, Solaris 9, Cygwin 1.7.x, mingw, MSVC 14, Android 4.3. But the provided replacement is just a dummy on some of these platforms: Minix 3.1.8. -@end itemize - -Portability problems not fixed by Gnulib: -@itemize @item This function prefers readers to writers (meaning, when this function is called on an rwlock that is already taken by one or more readers, and another writer is already waiting to take it, this function may return successfully immediately) -- a behaviour that may lead to writer starvation -- on some platforms: -glibc 2.28. +glibc 2.40. +@end itemize + +Portability problems not fixed by Gnulib: +@itemize @end itemize diff --git a/doc/posix-functions/pthread_rwlock_tryrdlock.texi b/doc/posix-functions/pthread_rwlock_tryrdlock.texi index a9611f53a0..e2d957c646 100644 --- a/doc/posix-functions/pthread_rwlock_tryrdlock.texi +++ b/doc/posix-functions/pthread_rwlock_tryrdlock.texi @@ -13,15 +13,15 @@ This function is missing on some platforms: Minix 3.1.8, mingw, MSVC 14, Android 4.3. But the provided replacement is just a dummy on some of these platforms: Minix 3.1.8. -@end itemize - -Portability problems not fixed by Gnulib: -@itemize @item This function prefers readers to writers (meaning, when this function is called on an rwlock that is already taken by one or more readers, and another writer is already waiting to take it, this function may return successfully immediately) -- a behaviour that may lead to writer starvation -- on some platforms: -glibc 2.28. +glibc 2.40. +@end itemize + +Portability problems not fixed by Gnulib: +@itemize @end itemize diff --git a/lib/pthread-rwlock.c b/lib/pthread-rwlock.c index cb972407db..46681538a3 100644 --- a/lib/pthread-rwlock.c +++ b/lib/pthread-rwlock.c @@ -49,6 +49,29 @@ pthread_rwlockattr_destroy (_GL_UNUSED pthread_rwlockattr_t *attr) return 0; } +#elif PTHREAD_RWLOCK_BAD_WAITQUEUE + +/* Override pthread_rwlockattr_init, to use the kind PREFER_WRITER_NONRECURSIVE + (or possibly PREFER_WRITER) instead of the kind DEFAULT. */ +int +pthread_rwlockattr_init (pthread_rwlockattr_t *attr) +# undef pthread_rwlockattr_init +{ + int err; + + err = pthread_rwlockattr_init (attr); + if (err != 0) + return err; + err = pthread_rwlockattr_setkind_np (attr, + PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP); + if (err != 0) + { + pthread_rwlockattr_destroy (attr); + return err; + } + return 0; +} + #endif #if (defined _WIN32 && ! defined __CYGWIN__) && USE_WINDOWS_THREADS @@ -363,7 +386,44 @@ pthread_rwlock_destroy (pthread_rwlock_t *lock) return 0; } -# elif PTHREAD_RWLOCK_LACKS_TIMEOUT +# else + +# if PTHREAD_RWLOCK_BAD_WAITQUEUE + +/* Override pthread_rwlock_init, to use the kind PREFER_WRITER_NONRECURSIVE + (or possibly PREFER_WRITER) instead of the default, when no + pthread_rwlockattr_t object is specified. */ +int +pthread_rwlock_init (pthread_rwlock_t *lock, const pthread_rwlockattr_t *attr) +# undef pthread_rwlock_init +{ + int err; + + if (attr != NULL) + err = pthread_rwlock_init (lock, attr); + else + { + pthread_rwlockattr_t replacement_attr; + + err = pthread_rwlockattr_init (&replacement_attr); + if (err != 0) + return err; + err = pthread_rwlockattr_setkind_np (&replacement_attr, + PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP); + if (err != 0) + { + pthread_rwlockattr_destroy (&replacement_attr); + return err; + } + err = pthread_rwlock_init (lock, &replacement_attr); + pthread_rwlockattr_destroy (&replacement_attr); + } + return err; +} + +# endif + +# if PTHREAD_RWLOCK_LACKS_TIMEOUT int pthread_rwlock_timedrdlock (pthread_rwlock_t *lock, @@ -479,6 +539,8 @@ pthread_rwlock_timedwrlock (pthread_rwlock_t *lock, } } +# endif + # endif #else diff --git a/lib/pthread.in.h b/lib/pthread.in.h index 81598616df..d32e79db92 100644 --- a/lib/pthread.in.h +++ b/lib/pthread.in.h @@ -1,4 +1,4 @@ -/* Implement the most essential subset of POSIX 1003.1-2008 pthread.h. +/* Implement the most essential subset of POSIX pthread.h. Copyright (C) 2009-2024 Free Software Foundation, Inc. @@ -275,7 +275,7 @@ typedef unsigned int pthread_rwlockattr_t; # undef PTHREAD_RWLOCK_INITIALIZER # define PTHREAD_RWLOCK_INITIALIZER { 0 } # endif -#elif @GNULIB_PTHREAD_RWLOCK@ && @REPLACE_PTHREAD_RWLOCK_INIT@ /* i.e. PTHREAD_RWLOCK_UNIMPLEMENTED */ +#elif @GNULIB_PTHREAD_RWLOCK@ && @REPLACE_PTHREAD_RWLOCK_DESTROY@ /* i.e. PTHREAD_RWLOCK_UNIMPLEMENTED */ # if @HAVE_PTHREAD_T@ # define pthread_rwlock_t rpl_pthread_rwlock_t # define pthread_rwlockattr_t rpl_pthread_rwlockattr_t @@ -296,6 +296,11 @@ typedef unsigned int pthread_rwlockattr_t; # undef PTHREAD_RWLOCK_INITIALIZER # define PTHREAD_RWLOCK_INITIALIZER \ { PTHREAD_MUTEX_INITIALIZER, PTHREAD_COND_INITIALIZER, PTHREAD_COND_INITIALIZER, 0, 0 } +#elif @GNULIB_PTHREAD_RWLOCK@ && @REPLACE_PTHREAD_RWLOCK_INIT@ /* i.e. PTHREAD_RWLOCK_BAD_WAITQUEUE */ +/* Use rwlocks of kind PREFER_WRITER or PREFER_WRITER_NONRECURSIVE instead of + the DEFAULT. */ +# undef PTHREAD_RWLOCK_INITIALIZER +# define PTHREAD_RWLOCK_INITIALIZER PTHREAD_RWLOCK_WRITER_NONRECURSIVE_INITIALIZER_NP #else # if @HAVE_PTHREAD_T@ # if !defined PTHREAD_RWLOCK_INITIALIZER && defined PTHREAD_RWLOCK_INITIALIZER_NP /* z/OS */ diff --git a/m4/pthread-rwlock.m4 b/m4/pthread-rwlock.m4 index 9592b2a5fb..51d84419c7 100644 --- a/m4/pthread-rwlock.m4 +++ b/m4/pthread-rwlock.m4 @@ -1,5 +1,5 @@ # pthread-rwlock.m4 -# serial 4 +# serial 5 dnl Copyright (C) 2019-2024 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -101,6 +101,362 @@ AC_DEFUN([gl_PTHREAD_RWLOCK], [Define if the functions pthread_rwlock_timedrdlock and pthread_rwlock_timedwrlock don't exist.]) ], [[#include ]]) + dnl In glibc ≥ 2.25 on Linux, test-pthread-rwlock-waitqueue reports + dnl "This implementation always prefers readers.", and this wait queue + dnl handling is unsuitable, because it leads to writer starvation: + dnl On machines with 8 or more CPUs, test-pthread-rwlock may never + dnl terminate. See + dnl + dnl + dnl for details. + AC_CACHE_CHECK([for reasonable pthread_rwlock wait queue handling], + [gl_cv_func_pthread_rwlock_good_waitqueue], + [case "$host_os" in + linux*-gnu*) + saved_LIBS="$LIBS" + LIBS="$LIBS $LIBPMULTITHREAD" + AC_RUN_IFELSE( + [AC_LANG_SOURCE([[ +/* This test is a simplified variant of tests/test-pthread-rwlock-waitqueue.c. */ +#include +#include +#include +#include +#include +#include + +#if defined __hppa +# define STEP_INTERVAL 20000000 /* nanoseconds */ +#elif defined __aarch64__ || defined __m68k__ +# define STEP_INTERVAL 10000000 /* nanoseconds */ +#else +# define STEP_INTERVAL 5000000 /* nanoseconds */ +#endif + +static pthread_rwlock_t lock; + +static pthread_rwlock_t sprintf_lock; + +struct locals +{ + const char *name; + unsigned int wait_before; + unsigned int wait_after; + char *result; +}; + +static void * +reader_func (void *arg) +{ + struct locals *l = arg; + int err; + + if (l->wait_before > 0) + { + struct timespec duration; + duration.tv_sec = l->wait_before / 1000000000; + duration.tv_nsec = l->wait_before % 1000000000; + nanosleep (&duration, NULL); + } + err = pthread_rwlock_rdlock (&lock); + if (err) + { + fprintf (stderr, "pthread_rwlock_rdlock failed, error = %d\n", err); + abort (); + } + if (pthread_rwlock_wrlock (&sprintf_lock)) + { + fprintf (stderr, "pthread_rwlock_wrlock on sprintf_lock failed\n"); + abort (); + } + sprintf (l->result + strlen (l->result), " %s", l->name); + if (pthread_rwlock_unlock (&sprintf_lock)) + { + fprintf (stderr, "pthread_rwlock_unlock on sprintf_lock failed\n"); + abort (); + } + if (l->wait_after > 0) + { + struct timespec duration; + duration.tv_sec = l->wait_after / 1000000000; + duration.tv_nsec = l->wait_after % 1000000000; + nanosleep (&duration, NULL); + } + err = pthread_rwlock_unlock (&lock); + if (err) + { + fprintf (stderr, "pthread_rwlock_unlock failed, error = %d\n", err); + abort (); + } + + return NULL; +} + +static void * +writer_func (void *arg) +{ + struct locals *l = arg; + int err; + + if (l->wait_before > 0) + { + struct timespec duration; + duration.tv_sec = l->wait_before / 1000000000; + duration.tv_nsec = l->wait_before % 1000000000; + nanosleep (&duration, NULL); + } + err = pthread_rwlock_wrlock (&lock); + if (err) + { + fprintf (stderr, "pthread_rwlock_rdlock failed, error = %d\n", err); + abort (); + } + if (pthread_rwlock_wrlock (&sprintf_lock)) + { + fprintf (stderr, "pthread_rwlock_wrlock on sprintf_lock failed\n"); + abort (); + } + sprintf (l->result + strlen (l->result), " %s", l->name); + if (pthread_rwlock_unlock (&sprintf_lock)) + { + fprintf (stderr, "pthread_rwlock_unlock on sprintf_lock failed\n"); + abort (); + } + if (l->wait_after > 0) + { + struct timespec duration; + duration.tv_sec = l->wait_after / 1000000000; + duration.tv_nsec = l->wait_after % 1000000000; + nanosleep (&duration, NULL); + } + err = pthread_rwlock_unlock (&lock); + if (err) + { + fprintf (stderr, "pthread_rwlock_unlock failed, error = %d\n", err); + abort (); + } + + return NULL; +} + +static const char * +do_test (const char *rw_string) +{ + size_t n = strlen (rw_string); + int err; + char resultbuf[100]; + + char **names = (char **) malloc (n * sizeof (char *)); + for (size_t i = 0; i < n; i++) + { + char name[10]; + sprintf (name, "%c%u", rw_string[i], (unsigned int) (i+1)); + names[i] = strdup (name); + } + + resultbuf[0] = '\0'; + + /* Create the threads. */ + struct locals *locals = (struct locals *) malloc (n * sizeof (struct locals)); + pthread_t *threads = (pthread_t *) malloc (n * sizeof (pthread_t)); + for (size_t i = 0; i < n; i++) + { + locals[i].name = names[i]; + locals[i].wait_before = i * STEP_INTERVAL; + locals[i].wait_after = (i == 0 ? n * STEP_INTERVAL : 0); + locals[i].result = resultbuf; + err = pthread_create (&threads[i], NULL, + rw_string[i] == 'R' ? reader_func : + rw_string[i] == 'W' ? writer_func : + (abort (), NULL), + &locals[i]); + if (err) + { + fprintf (stderr, "pthread_create failed to create thread %u, error = %d\n", + (unsigned int) (i+1), err); + abort (); + } + } + + /* Wait until the threads are done. */ + for (size_t i = 0; i < n; i++) + { + void *retcode; + err = pthread_join (threads[i], &retcode); + if (err) + { + fprintf (stderr, "pthread_join failed to wait for thread %u, error = %d\n", + (unsigned int) (i+1), err); + abort (); + } + } + + /* Clean up. */ + free (threads); + free (locals); + for (size_t i = 0; i < n; i++) + free (names[i]); + free (names); + + return strdup (resultbuf); +} + +static bool +startswith (const char *str, const char *prefix) +{ + return strncmp (str, prefix, strlen (prefix)) == 0; +} + +static int +find_wait_queue_handling (void) +{ + bool final_r_prefers_readers = true; + bool final_w_prefers_readers = true; + + /* Perform the test a few times, so that in case of a non-deterministic + behaviour that happens to look like deterministic in one round, we get + a higher probability of finding that it is non-deterministic. */ + for (int repeat = 3; repeat > 0; repeat--) + { + bool r_prefers_readers = false; + bool w_prefers_readers = false; + + { + const char * RWR = do_test ("RWR"); + const char * RWRR = do_test ("RWRR"); + const char * RWRW = do_test ("RWRW"); + const char * RWWR = do_test ("RWWR"); + const char * RWRRR = do_test ("RWRRR"); + const char * RWRRW = do_test ("RWRRW"); + const char * RWRWR = do_test ("RWRWR"); + const char * RWRWW = do_test ("RWRWW"); + const char * RWWRR = do_test ("RWWRR"); + const char * RWWRW = do_test ("RWWRW"); + const char * RWWWR = do_test ("RWWWR"); + + if ( startswith (RWR, " R1 R") + && startswith (RWRR, " R1 R") + && startswith (RWRW, " R1 R") + && startswith (RWWR, " R1 R") + && startswith (RWRRR, " R1 R") + && startswith (RWRRW, " R1 R") + && startswith (RWRWR, " R1 R") + && startswith (RWRWW, " R1 R") + && startswith (RWWRR, " R1 R") + && startswith (RWWRW, " R1 R") + && startswith (RWWWR, " R1 R")) + r_prefers_readers = true; + } + + { + const char * WRR = do_test ("WRR"); + const char * WRW = do_test ("WRW"); + const char * WWR = do_test ("WWR"); + const char * WRRR = do_test ("WRRR"); + const char * WRRW = do_test ("WRRW"); + const char * WRWR = do_test ("WRWR"); + const char * WRWW = do_test ("WRWW"); + const char * WWRR = do_test ("WWRR"); + const char * WWRW = do_test ("WWRW"); + const char * WWWR = do_test ("WWWR"); + const char * WRRRR = do_test ("WRRRR"); + const char * WRRRW = do_test ("WRRRW"); + const char * WRRWR = do_test ("WRRWR"); + const char * WRRWW = do_test ("WRRWW"); + const char * WRWRR = do_test ("WRWRR"); + const char * WRWRW = do_test ("WRWRW"); + const char * WRWWR = do_test ("WRWWR"); + const char * WRWWW = do_test ("WRWWW"); + const char * WWRRR = do_test ("WWRRR"); + const char * WWRRW = do_test ("WWRRW"); + const char * WWRWR = do_test ("WWRWR"); + const char * WWRWW = do_test ("WWRWW"); + const char * WWWRR = do_test ("WWWRR"); + const char * WWWRW = do_test ("WWWRW"); + const char * WWWWR = do_test ("WWWWR"); + + if ( startswith (WRR, " W1 R") + && startswith (WRW, " W1 R") + && startswith (WWR, " W1 R") + && startswith (WRRR, " W1 R") + && startswith (WRRW, " W1 R") + && startswith (WRWR, " W1 R") + && startswith (WRWW, " W1 R") + && startswith (WWRR, " W1 R") + && startswith (WWRW, " W1 R") + && startswith (WWWR, " W1 R") + && startswith (WRRRR, " W1 R") + && startswith (WRRRW, " W1 R") + && startswith (WRRWR, " W1 R") + && startswith (WRRWW, " W1 R") + && startswith (WRWRR, " W1 R") + && startswith (WRWRW, " W1 R") + && startswith (WRWWR, " W1 R") + && startswith (WRWWW, " W1 R") + && startswith (WWRRR, " W1 R") + && startswith (WWRRW, " W1 R") + && startswith (WWRWR, " W1 R") + && startswith (WWRWW, " W1 R") + && startswith (WWWRR, " W1 R") + && startswith (WWWRW, " W1 R") + && startswith (WWWWR, " W1 R")) + w_prefers_readers = true; + } + + final_r_prefers_readers &= r_prefers_readers; + final_w_prefers_readers &= w_prefers_readers; + } + + /* The wait queue handling is unsuitable if it always prefers readers, + because it leads to writer starvation: On machines with 8 or more CPUs, + test-pthread-rwlock may never terminate. */ + return final_r_prefers_readers && final_w_prefers_readers; +} + +int +main () +{ + /* Initialize the sprintf_lock. */ + if (pthread_rwlock_init (&sprintf_lock, NULL)) + { + fprintf (stderr, "pthread_rwlock_init failed\n"); + abort (); + } + + /* Find the wait queue handling of a default-initialized lock. */ + if (pthread_rwlock_init (&lock, NULL)) + { + fprintf (stderr, "pthread_rwlock_init failed\n"); + abort (); + } + { + int fail = find_wait_queue_handling (); + return fail; + } +} + ]]) + ], + [gl_cv_func_pthread_rwlock_good_waitqueue=yes], + [gl_cv_func_pthread_rwlock_good_waitqueue=no], + [dnl Guess no on glibc/Linux. + gl_cv_func_pthread_rwlock_good_waitqueue="guessing no" + ]) + LIBS="$saved_LIBS" + ;; + *) dnl Guess yes on other platforms. + gl_cv_func_pthread_rwlock_good_waitqueue="guessing yes" + ;; + esac + ]) + case "$gl_cv_func_pthread_rwlock_good_waitqueue" in + *yes) ;; + *no) + REPLACE_PTHREAD_RWLOCK_INIT=1 + REPLACE_PTHREAD_RWLOCKATTR_INIT=1 + AC_DEFINE([PTHREAD_RWLOCK_BAD_WAITQUEUE], [1], + [Define if the pthread_rwlock wait queue handling is not reasonable.]) + ;; + esac fi fi fi diff --git a/modules/pthread-rwlock b/modules/pthread-rwlock index 9f63b74a4d..c869f180cf 100644 --- a/modules/pthread-rwlock +++ b/modules/pthread-rwlock @@ -14,7 +14,7 @@ windows-timedrwlock [test $gl_threads_api = windows] configure.ac: gl_PTHREAD_RWLOCK gl_CONDITIONAL([GL_COND_OBJ_PTHREAD_RWLOCK], - [test $HAVE_PTHREAD_RWLOCK_TIMEDRDLOCK = 0 || test $REPLACE_PTHREAD_RWLOCK_TIMEDRDLOCK = 1]) + [test $REPLACE_PTHREAD_RWLOCK_INIT = 1 || test $HAVE_PTHREAD_RWLOCK_TIMEDRDLOCK = 0 || test $REPLACE_PTHREAD_RWLOCK_TIMEDRDLOCK = 1]) gl_PTHREAD_MODULE_INDICATOR([pthread-rwlock]) Makefile.am: