From 3dee29b6adb66ead5d09115357447c257dc3ac0f Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Thu, 12 Nov 2026 12:06:51 -0500 Subject: [PATCH v4 03/21] port: Fix assorted bugs in pg_threads.h. Presently, pg_mtx_recursive maps to the plain mutex type, so a recursive mutex is really a plain one and deadlocks on relock. No caller asks for one yet, which is why nobody noticed. pg_thrd_barrier_wait() reports an error to one waiter in every round on POSIX systems, because pthread_barrier_wait() returns PTHREAD_BARRIER_SERIAL_THREAD to that waiter. pgbench ignores the result, so this has been invisible so far. pg_thrd_map_pthread_error() formats its detail message with strerror(), which is pg_strerror() and uses a static buffer, so two threads failing at once race on it. Use strerror_r() instead. While at it, enlarge the detail buffer, which truncated the longer Windows messages, add the missing va_end() in pg_thrd_format_error(), and fix two macros that expanded to names no implementation defines: PG_TSS_DTOR_ITERATIONS and PG_RWLOCK_PLAIN_INIT. Reported-by: Jakub Wartak Discussion: https://postgr.es/m/CAKZiRmw3gpoXeOc9Pa_PRpfp-bTeQhg6gaY2Jw0Ck6NX4PWPhQ%40mail.gmail.com --- src/include/port/pg_threads.h | 4 ++-- src/include/port/pg_threads/map_pthread_ext.h | 16 ++++++++++++++-- src/include/port/pg_threads_ext.h | 2 +- src/port/pg_threads.c | 13 ++++++++++--- 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/include/port/pg_threads.h b/src/include/port/pg_threads.h index a5df4dc304d..1b1e18cdacf 100644 --- a/src/include/port/pg_threads.h +++ b/src/include/port/pg_threads.h @@ -95,13 +95,13 @@ enum enum { pg_mtx_plain = pg_mtx_plain_impl, - pg_mtx_recursive = pg_mtx_plain_impl, + pg_mtx_recursive = pg_mtx_recursive_impl, pg_mtx_timed = pg_mtx_timed_impl, }; /* standard macros with prefix */ #define PG_ONCE_FLAG_INIT PG_ONCE_FLAG_INIT_IMPL -#define PG_TSS_DTOR_ITERATIONS PG_TSS_DTOR_ITERARATIONS_IMPL +#define PG_TSS_DTOR_ITERATIONS PG_TSS_DTOR_ITERATIONS_IMPL /* standard cnd_t functions with prefix */ static inline int pg_cnd_broadcast(pg_cnd_t *cnd); diff --git a/src/include/port/pg_threads/map_pthread_ext.h b/src/include/port/pg_threads/map_pthread_ext.h index 3c591f15b55..4ebdc6bb5f6 100644 --- a/src/include/port/pg_threads/map_pthread_ext.h +++ b/src/include/port/pg_threads/map_pthread_ext.h @@ -47,7 +47,7 @@ enum #define PG_CND_INIT_IMPL PTHREAD_COND_INITIALIZER #define PG_MTX_PLAIN_INIT_IMPL PTHREAD_MUTEX_INITIALIZER #endif -#define PG_RWLOCK_INIT_IMPL PTHREAD_RWLOCK_INITIALIZER +#define PG_RWLOCK_PLAIN_INIT_IMPL PTHREAD_RWLOCK_INITIALIZER /* pg_rwlock_t */ PG_THREADS_FORWARD_VOID(pg_rwlock_destroy, pthread_rwlock_destroy, pg_rwlock_impl *); @@ -75,7 +75,19 @@ pg_thrd_barrier_init(pg_thrd_barrier_impl *barrier, int count) return pg_thrd_map(pthread_barrier_init(barrier, NULL, count)); } -PG_THREADS_MAP(pg_thrd_barrier_wait, pthread_barrier_wait, pg_thrd_barrier_impl *); +/* + * One waiter gets PTHREAD_BARRIER_SERIAL_THREAD instead of zero. We don't + * expose that distinction, so treat it as success. + */ +static inline int +pg_thrd_barrier_wait(pg_thrd_barrier_impl *barrier) +{ + int result = pthread_barrier_wait(barrier); + + if (result == PTHREAD_BARRIER_SERIAL_THREAD) + return pg_thrd_success_impl; + return pg_thrd_map(result); +} #endif #ifndef PG_THREADS_USE_THREADS_H diff --git a/src/include/port/pg_threads_ext.h b/src/include/port/pg_threads_ext.h index bebea5dfb09..0b5069a95fb 100644 --- a/src/include/port/pg_threads_ext.h +++ b/src/include/port/pg_threads_ext.h @@ -56,7 +56,7 @@ enum /* static initializer macros */ #define PG_CND_INIT PG_CND_INIT_IMPL #define PG_MTX_PLAIN_INIT PG_MTX_PLAIN_INIT_IMPL -#define PG_RWLOCK_PLAIN_INIT PG_RWLOCK_INIT_IMPL +#define PG_RWLOCK_PLAIN_INIT PG_RWLOCK_PLAIN_INIT_IMPL /* pg_rwlock_t functions */ static inline int pg_rwlock_init(pg_rwlock_t *lock, int type); diff --git a/src/port/pg_threads.c b/src/port/pg_threads.c index 57ae59e37f5..922060a2d5e 100644 --- a/src/port/pg_threads.c +++ b/src/port/pg_threads.c @@ -41,7 +41,7 @@ static int pg_thrd_format_error(const char *format, ...) pg_attribute_printf(1, 2); #ifdef PG_THREADS_HAVE_DETAILED_ERRORS -static thread_local char pg_thrd_last_error_detail[64]; +static thread_local char pg_thrd_last_error_detail[128]; #endif /* Convert the standard errors to fixed strings. */ @@ -110,6 +110,7 @@ pg_thrd_format_error(const char *format, ...) va_start(args, format); vsnprintf(pg_thrd_last_error_detail, sizeof(pg_thrd_last_error_detail), format, args); + va_end(args); #endif return pg_thrd_error; @@ -169,8 +170,14 @@ pg_thrd_map_pthread_error(int pthread_error) case ETIMEDOUT: return pg_thrd_timedout; default: - return pg_thrd_format_error("pthread error: %s", - strerror(pthread_error)); + { + char buffer[PG_STRERROR_R_BUFLEN]; + + return pg_thrd_format_error("pthread error: %s", + strerror_r(pthread_error, + buffer, + sizeof(buffer))); + } } } #endif -- 2.54.0