pg_threads.h take II

On 06.07.26 14:09, Thomas Munro wrote:

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

We still support platforms/compilers where it's not available, but

/messages/by-id/2a965ac6-fa42-4054-bee0-b1618e7729d6@eisentraut.org

which is ready to be committed, will de-support those, so by the time
this patch set is ready, we'll be fine.

On Tue, Jul 7, 2026 at 12:09 AM Thomas Munro <thomas.munro@gmail.com> wrote:

used here isn't what we want to go with for the future, but to do it
the way Heikki and Jelte are suggesting will probably require deciding
how to access atomics or at least compiler barrires in front-end code,
which I'll look into this week.

Correction: at least memory barriers

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:

/messages/by-id/CA+hUKGLtmexrpMtxBRLCVePqV_dtWG-ZsEbyPrYc+NBB2TkNsw@mail.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

On 7/6/26 3:48 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

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:

/messages/by-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

GetCurrentThread() not GetCurrentThreadId()...  GetCurrentThread()
returns a special sentinel handle (-2) that when passed to another
Win32 function causes the kernel to resolve to whatever thread made
the call....

So, no-- not broken.

On 07/07/2026 00:01, dbryan.green@gmail.com wrote:

On 7/6/26 3:48 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

On 06/07/2026 15:09, Thomas Munro wrote:

+#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?

GetCurrentThread() not GetCurrentThreadId()...  GetCurrentThread()
returns a special sentinel handle (-2) that when passed to another
Win32 function causes the kernel to resolve to whatever thread made
the call....

So, no-- not broken.

Ah gotcha. Could we use GetCurrentThreadId() here then? (I have no
problem with the way it's currently done in the patch either though,
just curious)

- Heikki

On 7/6/2026 4:13 PM, Heikki Linnakangas wrote:

On 07/07/2026 00:01, dbryan.green@gmail.com wrote:

On 7/6/26 3:48 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

On 06/07/2026 15:09, Thomas Munro wrote:

+#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?

GetCurrentThread() not GetCurrentThreadId()...  GetCurrentThread()
returns a special sentinel handle (-2) that when passed to another
Win32 function causes the kernel to resolve to whatever thread made
the call....

So, no-- not broken.

Ah gotcha. Could we use GetCurrentThreadId() here then? (I have no
problem with the way it's currently done in the patch either though,
just curious)

- Heikki

I haven't looked at all of the code, but I would assume there is a
WaitFor...Object() somewhere and those only work with actual
handles...not id's. GetCurrentThreadId() returns a dword for id. I'll
look over the patches as well.

--
Bryan Green
EDB: https://www.enterprisedb.com