Cleaning up threading code

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Cleaning up threading code
Date: 2023-06-10 02:23:52
Message-ID: CA+hUKGLtmexrpMtxBRLCVePqV_dtWG-ZsEbyPrYc+NBB2TkNsw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

This message is not about inventing backend threads, but I was
reminded to post this by some thought-provoking hallway track
discussion of that large topic at PGCon, mostly with Heikki.

1. We should completely remove --disable-thread-safety and the
various !defined(ENABLE_THREAD_SAFETY) code paths. There are no
computers that need it, it's not being tested, and support was already
dropped/hardcoded in meson.build. Here is a mostly mechanical patch
to finish that job, originally posted in one of the big 'historical
baggage' threads -- I just didn't get around to committing in time for
16. Also a couple of related tiny cleanups for port/thread.c, which
is by now completely bogus.

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

Here is an attempt at that last thing. Since I don't really like
making up new names for things, I just used all the C11 names but with
pg_ in front, and declared it all in "port/pg_threads.h" (actually I
tried to write C11 replacements and then noped out and added the pg_
prefixes). I suppose the idea is that it and the prefixes might
eventually go away. Note: here I am talking only about very basic
operations like create, exit, join, explicit thread locals -- the
stuff that we actually use today in frontend code. I'm not talking
about other stuff like C11 atomics, memory models, and the
thread_local storage class, which are all very good and interesting
topics for another day.

One mystery still eludes me on Windows: while trying to fix the
ancient bug that ECPG leaks memory on Windows, because it doesn't call
thread-local destructors, I discovered that if you use FlsAlloc
instead of TlsAlloc you can pass in a destructor (that's
"fibre-local", but we're not using fibres so it's works out the same
as thread local AFAICT). It seems to crash in strange ways if you
uncomment the line FlsAlloc(destructor). Maybe it calls the
destructor even for NULL value? Or maybe it suffers from some kind of
Windowsian multiple-heaps-active problem. I noticed that some other
attempts at implementing tss on Windows use TlsAlloc but do their own
bookkeeping of destructors and run them at thread exit, which I was
hoping to avoid, but ...

Before I spend time fixing that issue or seeking Windowsian help...
do you think this direction is an improvement?

Attachment Content-Type Size
0001-Remove-obsolete-comments-and-code-from-fe-auth.c.patch text/x-patch 1.3 KB
0002-Rename-port-thread.c-to-port-user.c.patch text/x-patch 2.7 KB
0003-Remove-disable-thread-safety.patch text/x-patch 59.4 KB
0004-Add-port-pg_threads.h-for-a-common-threading-API.patch text/x-patch 10.1 KB
0005-Replace-various-ad-hoc-abstractions-with-pg_threads..patch text/x-patch 62.9 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message James Coleman 2023-06-10 02:26:54 Add last_commit_lsn to pg_stat_database
Previous Message Joe Conway 2023-06-10 02:10:20 Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG