Re: Cleaning up threading code

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: Peter Eisentraut <peter(at)eisentraut(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Cleaning up threading code
Date: 2024-04-14 03:16:56
Message-ID: CA+hUKG+XpCcqoNe3-3oUzuGUeRoxAKqGnEAedvOhotsNCxuS+Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jul 3, 2023 at 8:43 PM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> On 10/06/2023 05:23, Thomas Munro wrote:
> > 2. I don't like the way we have to deal with POSIX vs Windows at
> > every site where we use threads, and each place has a different style
> > of wrappers. I considered a few different approaches to cleaning this
> > up:
> >
> > * provide centralised and thorough pthread emulation for Windows; I
> > don't like this, I don't even like all of pthreads and there are many
> > details to get lost in
> > * adopt C11 <threads.h>; unfortunately it is too early, so you'd need
> > to write/borrow replacements for at least 3 of our 11 target systems
> > * invent our own mini-abstraction for a carefully controlled subset of stuff
>
> Google search on "c11 threads on Windows" found some emulation wrappers:
> https://github.com/jtsiomb/c11threads and
> https://github.com/tinycthread/tinycthread, for example. Would either of
> those work for us?
>
> Even if we use an existing emulation wrapper, I wouldn't mind having our
> own pg_* abstration on top of it, to document which subset of the POSIX
> or C11 functions we actually use.

Yeah. I am still interested in getting our thread API tidied up, and
I intend to do that for PG 18. The patch I posted on that front,
which can be summarised as a very lightweight subset of standard
<threads.h> except with pg_ prefixes everywhere mapping to Windows or
POSIX threads, still needs one tiny bit more work: figuring out how to
get the TLS destructors to run on Windows FLS or similar, or
implementing destructors myself (= little destructor callback list
that a thread exit hook would run, work I was hoping to avoid by using
something from the OS libs... I will try again soon). Andres also
opined upthread that we should think about offering a thread_local
storage class and getting away from TLS with keys.

One thing to note is that the ECPG code is using TLS with destructors
(in fact they are b0rked in master, git grep "FIXME: destructor" so
ECPG leaks memory on Windows, so the thing that I'm trying to fix in
pg_threads.h is actually fixing a long-standing bug), and although
thread_local has destructors in C++ it doesn't in C, so if we decided
to add the storage class but not bother with the tss_create feature,
then ECPG would need to do cleanup another way. I will look into that
option.

One new development since last time I wrote the above stuff is that
the Microsoft toolchain finally implemented the library components of
C11 <threads.h>:

https://devblogs.microsoft.com/cppblog/c11-threads-in-visual-studio-2022-version-17-8-preview-2/

It seems like it would be a long time before we could contemplate
using that stuff though, especially given our duty of keeping libpq
and ECPG requirements low and reasonable. However, it seems fairly
straightforward to say that we require C99 + some way to access a
C11-like thread local storage class. In practice that means a
pg_thread_local macro that points to MSVC __declspec(thread) (which
works since MSVC 2014?) or GCC/Clang __thread_local (which works since
GCC4.8 in 2013?) or whatever. Of course every serious toolchain
supports this stuff somehow or other, having been required to for C11.

I can't immediately see any build farm animals that would be affected.
Grison's compiler info is out of date, it's really running
8.something. The old OpenBSD GCC 4.2 animal was upgraded, and antique
AIX got the boot: that's not even a coincidence, those retirements
came about because those systems didn't support arbitrary alignment,
another C11 feature that we now effectively require. (We could have
worked around it it we had to but on but they just weren't reasonable
targets.)

So I'll go ahead and add the storage class to the next version, and
contemplate a couple of different options for the tss stuff, including
perhaps leaving it out if that seems doable.

https://stackoverflow.com/questions/29399494/what-is-the-current-state-of-support-for-thread-local-across-platforms

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Wen Yi 2024-04-14 07:47:55 Re: Things I don't like about \du's "Attributes" column
Previous Message David Rowley 2024-04-14 03:14:46 SupportRequestRows support function for generate_series_timestamptz