On 06/07/2026 15:09, Thomas Munro wrote:
Hi,
Here's another go at port/pg_threads.h, a C11 <threads.h>-like
interface, with some patches to use it (some just sketch-quality).
Previous work:
https://www.postgresql.org/message-id/flat/CA%2BhUKGLtmexrpMtxBRLCVePqV_dtWG-ZsEbyPrYc%2BNBB2TkNsw%40mail.gmail.com
We still can't require C11 <threads.h> yet, though the interface has a
clear future upgrade/remap path to do that eventually. Since I last
wrote about this topic, we gained the ability to count on C11
thread_local being available (it's a language/compiler feature and
doesn't need a <threads.h> header or library), and partly because of
that, but also because I don't think anyone really likes it, I ripped
out that tss_t stuff. I came up with a narrower API to get
thread-exit cleanup callbacks if you need them for your thread_local
resources, same pthread_key_t/FlsAlloc stuff underneath it but without
having to get lost in the weeds of <threads.h> conformance.
Nice!
I've now reviewed patches 0001-0007. I think they're ready to be
committed, with some small comments below:
v1-0002-port-Provide-minimal-pg_threads.h-API.patch
The main thing I'm wondering about is how to make the error reporting
a bit nicer. Neither pthreads nor C11 threads set errno, but it's
nice if you can use %m. Hmm.
Perhaps provide a function like:
void
set_errno_from_pg_thrd_error(int error)
{
switch((pg_thrd_error_t) error)
{
case pg_thrd_nomem:
errno = ENOMEM;
break;
case pg_thrd_busy:
errno = EBUSY;
break;
...
}
}
+/* Convert native error to pg_thrd_error_t. */
+static inline int
+pg_thrd_maperror(int error)
+{
+#ifdef WIN32
+ return error ? pg_thrd_success : pg_thrd_error;
+#else
+ return error == 0 ? pg_thrd_success : pg_thrd_error;
+#endif
+}
The WIN32 version of this is confusing. There is only one caller of this
with WIN32:
+#elif defined(WIN32)
+ return pg_thrd_maperror(InitializeSynchronizationBarrier(barrier,
count, 0));
+#else
So that's not wrong. But I'd suggest only defining pg_thrd_maperror()
with pthreads, and handling that one WIN32 caller directly without the
helper function. Maybe rename pg_thrd_maperror() to something like
map_pthread_retval_to_pg_thrd_error() or something.
Do the pthread functions return an errno on error? Currently, all errors
get squashed to pg_thrd_error, which is a little unfortunate.
+/* Like C11 mtx_type_t. */
+typedef enum pg_mtx_type_t
+{
+ pg_mtx_plain = 0
+} pg_mtx_type_t;
+
+/* Like C11 mtx_init(). */
+static inline int
+pg_mtx_init(pg_mtx_t *mutex, int type)
+{
+#ifdef WIN32
+ return pg_rwlock_init(mutex);
+#else
+ return pg_thrd_maperror(pthread_mutex_init(mutex, NULL));
+#endif
+}
Since we only support plain mutexes, how about "Assert(type ==
pg_mtx_plan)" here?
+/*-------------------------------------------------------------------------
+ *
+ * Barriers. Not in C11.
+ *
+ *-------------------------------------------------------------------------
+ */
I wonder if we really need barriers. They're not that useful IMHO. I'd
tend to just open-code this directly with a mutex and the condition
variable in most cases. (Not a strong objection, there's little harm in
having it either)
+/*
+ * Wait for all expected threads to arrive at the barrier, and elect one
+ * arbitrary thread to perform a phase of computation serially. Sets
+ * *elected_thread to true in the elected thread, and false in all others.
+ */
+static inline int
+pg_barrier_wait_and_elect(pg_barrier_t *barrier, bool *elected_thread)
+{
No callers use the *elected_thread return value. What was the idea here?
+#ifdef WIN32
+
+ /*
+ * Retrieve handle passed here by pg_thrd_create() before allowing this
+ * thread to run. (pg_thrd_current() can't use CurrentThread(), because
+ * that returns a pseudo-handle with the same value in all threads.)
+ */
+ Assert(start_info->self);
+ my_thrd_handle = start_info->self;
+#endif
Does that refer to the GetCurrentThreadId() function? We use that in a
few places currently.
How can it return the same value in all threads, isn't that completely
useless? And does that mean all our current uses of it are broken?
- Heikki
