| From: | Yuriy Osadchyy <yuriy_osadchyy(at)trimble(dot)com> |
|---|---|
| To: | pgsql-bugs(at)lists(dot)postgresql(dot)org |
| Cc: | Yuriy Osadchyy <yuriy_osadchyy(at)trimble(dot)com> |
| Subject: | data race in libpq process-global variables (static_client_encoding / static_std_strings) when connections are established concurrently |
| Date: | 2026-10-21 13:44:42 |
| Message-ID: | CALTrLdL0U90tmUjDUvEYcDO8JbA-yrNrTeZeNEmvBwX9OgQVOg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
Dear PostgreSQL hackers,
While running ThreadSanitizer on a multithreaded application that
establishes PostgreSQL connections from multiple threads (each thread
creating its own connections, as documented as the supported pattern
for libpq), TSan reports data races in libpq itself. The races are on
two process-global variables in src/interfaces/libpq/fe-exec.c,
written by pqSaveParameterStatus() during the startup of every
PQconnectdb() call:
/*
* static state needed by PQescapeString and PQescapeBytea;
* initialize to values that result in backward-compatible behavior
*/
static int static_client_encoding = PG_SQL_ASCII;
static bool static_std_strings = false;
...
/* inside pqSaveParameterStatus() */
if (strcmp(name, "client_encoding") == 0)
{
conn->client_encoding = pg_char_to_encoding(value);
...
static_client_encoding = conn->client_encoding; /* unsynchronized
*/
}
else if (strcmp(name, "standard_conforming_strings") == 0)
{
conn->std_strings = (strcmp(value, "on") == 0);
static_std_strings = conn->std_strings; /* unsynchronized
*/
}
Two threads running PQconnectdb() concurrently write these globals
without synchronization (and PQescapeString()/PQescapeBytea() read
them), which is undefined behavior per the C memory model. This is
present in libpq 16.9 and is still present on master as of
2026-10-16 (checked the source directly).
The libpq documentation states that libpq is "always reentrant and
thread-safe" and that "it is possible to use libpq in a multithreaded
program, as long as ... each connection is used by only one thread at
a time" ("Using libpq in a Multi-threaded Program"). Concurrent
establishment of multiple connections is the common pattern that
documentation describes, so the process-global state above appears to
contradict the documented thread-safety guarantee.
Minimal reproduction (no server data required)
----------------------------------------------
Any recent PostgreSQL server is enough; only the startup
ParameterStatus exchange matters (a single-threaded fake server
implementing AuthenticationOk, ParameterStatus x N, BackendKeyData,
ReadyForQuery works too).
repro.c (compile with -fsanitize=thread, link against libpq):
#include <libpq-fe.h>
#include <pthread.h>
#include <stdio.h>
#define NTHREADS 16
#define NCONNS 300
static const char *DSN = "host=127.0.0.1 port=5432 dbname=repro
user=repro";
static void *worker(void *arg)
{
(void) arg;
long ok = 0;
for (int i = 0; i < NCONNS; i++)
{
PGconn *c = PQconnectdb(DSN);
if (PQstatus(c) == CONNECTION_OK)
ok++;
PQfinish(c);
}
return (void *) ok;
}
int main(void)
{
pthread_t t[NTHREADS];
long total = 0;
for (int i = 0; i < NTHREADS; i++)
pthread_create(&t[i], 0, worker, 0);
for (int i = 0; i < NTHREADS; i++)
{
void *r;
pthread_join(t[i], &r);
total += (long) r;
}
printf("connected_ok=%ld expected=%d\n", total, NTHREADS * NCONNS);
return total == (long) NTHREADS * NCONNS ? 0 : 2;
}
ThreadSanitizer output (excerpts)
---------------------------------
Local run of the repro above: libpq 16.9 built from the official
tarball with -fsanitize=thread -g -O1 -fno-omit-frame-pointer
(gcc 13, Ubuntu 24.04), 16 threads x 300 PQconnectdb() each,
connected_ok=4800/4800. TSan reported exactly two races, both in
pqSaveParameterStatus():
WARNING: ThreadSanitizer: data race
Write of size 4 at 0x5555555ab0ac by thread T6:
#0 pqSaveParameterStatus src/interfaces/libpq/fe-exec.c:1133
#1 getParameterStatus src/interfaces/libpq/fe-protocol3.c:1462
#2 pqParseInput3 src/interfaces/libpq/fe-protocol3.c:280
#3 parseInput src/interfaces/libpq/fe-exec.c:1997
#4 PQisBusy src/interfaces/libpq/fe-exec.c:2012
#5 PQconnectPoll src/interfaces/libpq/fe-connect.c:3849
#6 connectDBComplete src/interfaces/libpq/fe-connect.c:2511
#7 PQconnectdb src/interfaces/libpq/fe-connect.c:738
#8 worker repro.c:11
Previous write of size 4 at 0x5555555ab0ac by thread T1:
#0 pqSaveParameterStatus src/interfaces/libpq/fe-exec.c:1133
... (same stack, different thread) ...
Location is global 'static_client_encoding' of size 4
SUMMARY: ThreadSanitizer: data race fe-exec.c:1133 in
pqSaveParameterStatus
(Second report, same shape - write/write between two connecting
threads, on the other global:)
Write of size 1 at 0x5555555ab0a8 by thread T6:
#0 pqSaveParameterStatus src/interfaces/libpq/fe-exec.c:1138
... (same stack, different thread) ...
Location is global 'static_std_strings' of size 1
SUMMARY: ThreadSanitizer: data race fe-exec.c:1138 in
pqSaveParameterStatus
ThreadSanitizer: reported 2 warnings
Suggested fix
-------------
Make the two globals relaxed atomics. They are only read by the
deprecated connectionless PQescapeString()/PQescapeBytea(), whose
documented behavior is to mirror the most recently established
connection's settings; keeping the writes as relaxed atomic stores
preserves that behavior exactly while the race (and the UB) goes away.
A candidate patch against 16.9 is attached: the globals become
"static volatile uint32" (zero-initialization equals the current
PG_SQL_ASCII / false initializers, since PG_SQL_ASCII is the first
pg_encoding enumerator), and all four accesses go through a small
two-function shim in the same file:
#ifdef _MSC_VER
/* store: _InterlockedExchange; load: aligned volatile 32-bit read
*/
#else
/* store: __atomic_store_n(p, v, __ATOMIC_RELAXED)
load: __atomic_load_n(p, __ATOMIC_RELAXED) */
#endif
Implementation notes that may matter for how this lands in-tree:
- C11 atomics are not usable in fe-exec.c on all supported toolchains.
MSVC does not support C11 atomics in C mode (verified with VS 17.13
/ cl 14.43: default mode rejects _Atomic with "C atomics require
C11 or later"; /std:c11 defines __STDC_NO_ATOMICS__); Microsoft
exposes the feature only behind /experimental:c11atomics, which
PostgreSQL's MSBuild does not pass. The GCC/Clang __atomic builtins
above are what C11 relaxed atomics compile to, and they are
available in plain C mode.
- pg_atomic_* is not usable in libpq either: port/atomics.h currently
contains "#ifdef FRONTEND #error" (libpq is frontend code), and on
x86-64 the u32 read/write implementations are plain hardware
accesses (generic.h falls back to plain load/store), which
sanitizer-instrumented builds still flag as races.
A less invasive alternative is to make the globals const and remove
the two writes: that also removes the UB, but it changes the behavior
of the deprecated connectionless PQescape*() entry points (they would
keep their compile-time defaults instead of mirroring the last
connection), so the atomic version seems preferable.
Impact
------
- Undefined behavior (data race) in a documented, supported usage
pattern.
- TSan-instrumented builds of any client that opens connections
concurrently are unreliable (false positives mask real races, or
abort the process).
- The values involved (an encoding id, a bool) make the practical
risk low, but the UB is real: nothing guarantees the accesses
behave as observed once they are unsynchronized.
We apply the candidate patch locally (vcpkg overlay port) in the
meantime, and are happy to provide the patch in git format-patch form
if that is easier to review.
Thanks,
Yuriy Osadchyy
| Attachment | Content-Type | Size |
|---|---|---|
| fe-exec-threadsafety.patch | application/octet-stream | 3.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Fujii Masao | 2026-10-21 14:47:53 | Re: Inconsistency in psql output with \d+x PATTERN |
| Previous Message | Nitin Motiani | 2026-10-21 10:29:55 | Re: BUG #19765: Wrong result from a chain of LEFT JOINs since v16 |