Re: pg_threads.h take II

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Bryan Green <dbryan(dot)green(at)gmail(dot)com>
Cc: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Jelte Fennema <postgres(at)jeltef(dot)nl>
Subject: Re: pg_threads.h take II
Date: 2026-07-30 06:13:54
Message-ID: CA+hUKG+TxTNMhdYMumTHnzBg6CqJUJxZxUheOxj5LfdnoNxSzw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jul 7, 2026 at 8:48 AM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> 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;
> ...
> }
> }

I thought a lot more about error reporting, standards conformance,
expected future evolution and maintainability, and now I have a more
developed version that tries harder to expose underlying error
conditions.

> Since we only support plain mutexes, how about "Assert(type ==
> pg_mtx_plan)" here?

It returns an error for bogus type values now, and also supports all
mutex types.

> 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)

It's just that pgbench is using them. If we boot them out of here
then it'll need to be defined there instead, and then either you won't
get the native one on most systems (which might be slightly better in
some way?), or native threading code will once again roam the tree...
I think this is OK like this, but:

> No callers use the *elected_thread return value. What was the idea here?

I have removed that argument for now, as pgbench doesn't use it. I
just wanted to expose the pthread/Windows special result for that,
which involved making up names. One of the goals of following
<threads.h> was not to have to make up names for things so I'm happy
to remove that bit :-)

Speaking of making names up, I'm also trying the name
pg_thrd_barrier_t rather than pg_barrier_t, as that felt a little
vague/overloaded. (Someone suggested that but I've lost track of
who/where, sorry...)

On Tue, Jul 7, 2026 at 9:28 AM Bryan Green <dbryan(dot)green(at)gmail(dot)com> wrote:
> 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.

Yeah. And once I tackled thrd_detach() as part of my API completeness
drive, I realised we really need both. See details in patch.

Changes in this version:

* pg_threads.h has 100% of the <threads.h> API covered, and none of
our extensions.

* pg_threads_ext.h now has the (few) extra things that we want as of
today on top of that. Now it's much clearer when we're inventing
things and when we're following a standard to the letter.

* The per-platform implementations are now in separate files
pg_threads/map_pthread.h, .../map_threads.h and .../map_windows.h.

* That second implementation, for system-provided <threads.h> option,
is a new addition.

* Now that we require Visual Studio 2022, I think we might as well
actually use the <threads.h> implementation when we detect Visual
Studio? Technically it's 17.8 (not 17.0) that introduced it, but
that's out of vendor support. But if that's a problem, perhaps the
test could be not just defined(_MSC_VER) but also _MSC_VER >= 1938.
(See also question about associated DLL at end.)

* We still need map_windows.h for MinGW, either way. At some point it
will hopefully have <threads.h>, and then we could consider deleting
map_windows.h and most of pg_threads.c. That'd leave very little
code, as paper-thin <threads.h> and <pthread.h> mappings are mostly
just inline renaming wrappers + the small extensions.

* I think it's very useful to be able to test <threads.h> without
Windows, so I made it so that you can define PG_THREADS_USE_THREADS_H
manually. That's a little tricky because no standard tells us how to
define static initializers for cnd_t and mtx_t (though fortunately we
do have authoritative values for Windows). It's easy in practice on
at least Linux and FreeBSD though, so I tried to come up with enough
disclaimers and assertions to get away with that as a developer-only
option.

* I studied the Windows handle-vs-ID stuff a bit more and realised
that I'd actually solved a conformance problem that Visual Studio's
own <threads.h> suffers from. On the other hand, that has a runtime
cost, which I assume its authors declined to pay. Real programs
almost certainly don't care about that finer point anyway (namely
threads detaching themselves or passing their own thrd_t to another
thread for that thread to join). So now I just do it the same
non-conforming way as Visual Studio. I have left the code present as
an option guarded with PG_THRD_CURRENT_CONFORMING just to illustrate
what's going on here; unless anyone sees any reason not to, I'll
probably delete that code.

* I think I see why macOS hasn't given us <threads.h> yet: its
<pthread.h> mutexes don't support timeouts (the <unistd.h>
_POSIX_TIMEOUTS feature set). Those operations will simply fail. At
some point, Apple will presumably solve this problem and give us
<threads.h>, or if they don't, if we actually really want timeouts we
could build our own from lower level nuts and bolts and use that when
you ask for pg_mtx_timed, but for now pg_mtx_init(..., pg_mtx_timed)
and pg_mtx_timedwait() fail with an unsupported message and the macro
PG_MTX_TIMED_NOT_SUPPORTED is defined. I don't think it's much of a
problem in practice. pg_mtx_trylock() works everywhere, and is
probably more useful in real programs.

* map_windows.h has the same problem: no timeouts with any of the
basic Win32 APIs (well I can see how to do that, but then you lose
other things, I wrote some details in a comment). Visual Studio's
<threads.h> solved that problem in its mtx_t, so is more conforming in
that respect (I declined to write a low level mtx_t and cnd_t
operation for now... I mean it's not that hard, but right now we
don't need it).

* I realised we don't really need a configure check for
pthread_barrier_wait, because <unistd.h> advertises _POSIX_BARRIERS
for that purpose (another macOS-only problem, POSIX:2018 made it
non-optional).

* Patches 0009-0011 are the pg_dump change I hack on a few weeks ago,
an area Bryan is actively working on. He's gone further than me with
the in-memory queue stuff that we clearly want, so let's focus on his
patches for pg_dump (I will write about that soon). I'm just
including a rebase of my earlier attempt because it exercises and
demonstrates API usage...

* I dislike synthesised/translated OS errors, and the interesting case
thrd_error has no translation anyway. I came up with new extended
error message retrieval API and put it in pg_thread_ext.h where our
other API extensions live.

Example of error reporting:

if ((error = pg_thrd_create(...)) != pg_thrd_success)
pg_fatal("could not create thread: %s",
pg_thrd_error_string_with_detail(error));

It regurgitates platform-specific information which lives in a
thread-local buffer referring to the last failure, much like the scope
of errno and GetLastError():

could not create thread: Windows error 1234
could not create thread: Operation not permitted

It's a string because I also wanted implementations to have the option
of adding context about internal checks or the system call that failed
in multi-step operations:

pg_mtx_init(): bad type flags 42
pg_thrd_join(): no handle for thread 14304
pg_thrd_join(): GetExitCodeThread() failed: Windows error 259

The motivation for collecting as much information as possible is that
I'm imagining the sort of Windows issues or
running-in-container-with-weird-limits errors that show up on the
list. Just converting pg_thrd_error -> EINVAL or whatever bogus value
you want to pick seems like a bad plan in that context.

When using system-provided <threads.h> that isn't available so you
just get 5 pre-baked error strings, but there isn't much we can do
about that. At least if map_windows.h contains bugs then the detailed
error messages might help us find them.

About using system <threads.h> on Windows:

I'm unsure about the packaging/distribution implications. The runtime
code for that seems to be in a new library vcruntime140_threads.dll.
If it's statically linked there should be no issue, but otherwise, I
guess it might need to be distributed, like I guess vcruntime140.dll
is? I just know that it works on CI, need to learn more about that...

We could also decide that it's too soon to pull that trigger, and keep
map_threads.h as a working but not-yet-selected-by-default option on
any OS. Even if we don't use it yet, I like the fact that it acts as
proof that our pg_threads.h models <threads.h> exactly, with
systematic name prefixes.

(Perhaps the code in vrtunetime140_threads.dll will eventually migrate
to UCRT, I mean it *is* C runtime code, one would think. I wonder if
that's what MinGW needs to happen before it can supply <threads.h>,
but I have no clues about any of that.)

Attachment Content-Type Size
v2-0001-ecpg-Fix-auto_mem-cleanup-on-thread-exit.patch text/x-patch 2.2 KB
v2-0002-port-Provide-pg_threads.h-API.patch text/x-patch 69.9 KB
v2-0003-port-Use-pg_threads.h-API-for-pg_localconv_r.patch text/x-patch 1.6 KB
v2-0004-ecpg-Improve-variable-name.patch text/x-patch 2.5 KB
v2-0005-ecpg-Use-pg_threads.h.patch text/x-patch 53.2 KB
v2-0006-pgbench-Use-pg_threads.h.patch text/x-patch 13.9 KB
v2-0007-libpq-Use-pg_threads.h.patch text/x-patch 11.6 KB
v2-0008-pg_basebackup-Use-pg_threads.h.patch text/x-patch 12.4 KB
v2-0009-pg_signal_processor-Infrastructure-for-cleanup.patch text/x-patch 7.9 KB
v2-0010-fe_utils-Provide-cancel_set-for-fast-quit-paths.patch text/x-patch 6.2 KB
v2-0011-XXX-pg_dump-Refactor-to-use-threads-on-all-platfo.patch text/x-patch 46.6 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Nisha Moond 2026-07-30 06:19:05 Re: Support EXCEPT for TABLES IN SCHEMA publications
Previous Message Nisha Moond 2026-07-30 06:04:51 Re: Support EXCEPT for TABLES IN SCHEMA publications