v1-0002-Give-fmtId-s-temporary-buffer-thread-local-storag.patch
text/plain
Filename: v1-0002-Give-fmtId-s-temporary-buffer-thread-local-storag.patch
Type: text/plain
Part: 1
Patch
Format: format-patch
Series: patch v1-0002
Subject: Give fmtId()'s temporary buffer thread-local storage
| File | + | − |
|---|---|---|
| src/bin/pg_dump/parallel.c | 0 | 52 |
| src/fe_utils/string_utils.c | 4 | 3 |
From 3c1b8510a546a504dd583c7f0979f645c30b8ac6 Mon Sep 17 00:00:00 2001
From: Bryan Green <dbryan.green@gmail.com>
Date: Sun, 28 Jun 2026 11:40:58 -0500
Subject: [PATCH v1 2/4] Give fmtId()'s temporary buffer thread-local storage
getLocalPQExpBuffer() returns a function-scope static buffer, unsafe when
fmtId() and fmtQualifiedId() run in multiple threads. On Windows, where
pg_dump's parallel workers are threads, this was patched over at runtime
by swapping in a TLS-based buffer from ParallelBackupStart().
Mark the default buffer C11 _Thread_local instead. Each thread gets its
own, so the Windows TlsAlloc/TlsGetValue workaround and the
getLocalPQExpBuffer function-pointer swap can go.
---
src/bin/pg_dump/parallel.c | 52 -------------------------------------
src/fe_utils/string_utils.c | 7 ++---
2 files changed, 4 insertions(+), 55 deletions(-)
diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.c
index b77d2650df..12b462375d 100644
--- a/src/bin/pg_dump/parallel.c
+++ b/src/bin/pg_dump/parallel.c
@@ -193,9 +193,6 @@ static CRITICAL_SECTION signal_info_lock;
#ifdef WIN32
-/* file-scope variables */
-static DWORD tls_index;
-
/* globally visible variables (needed by exit_nicely) */
bool parallel_init_done = false;
DWORD mainThreadId;
@@ -243,8 +240,6 @@ init_parallel_dump_utils(void)
WSADATA wsaData;
int err;
- /* Prepare for threaded operation */
- tls_index = TlsAlloc();
mainThreadId = GetCurrentThreadId();
/* Initialize socket access */
@@ -280,48 +275,6 @@ GetMyPSlot(ParallelState *pstate)
return NULL;
}
-/*
- * A thread-local version of getLocalPQExpBuffer().
- *
- * Non-reentrant but reduces memory leakage: we'll consume one buffer per
- * thread, which is much better than one per fmtId/fmtQualifiedId call.
- */
-#ifdef WIN32
-static PQExpBuffer
-getThreadLocalPQExpBuffer(void)
-{
- /*
- * The Tls code goes awry if we use a static var, so we provide for both
- * static and auto, and omit any use of the static var when using Tls. We
- * rely on TlsGetValue() to return 0 if the value is not yet set.
- */
- static PQExpBuffer s_id_return = NULL;
- PQExpBuffer id_return;
-
- if (parallel_init_done)
- id_return = (PQExpBuffer) TlsGetValue(tls_index);
- else
- id_return = s_id_return;
-
- if (id_return) /* first time through? */
- {
- /* same buffer, just wipe contents */
- resetPQExpBuffer(id_return);
- }
- else
- {
- /* new buffer */
- id_return = createPQExpBuffer();
- if (parallel_init_done)
- TlsSetValue(tls_index, id_return);
- else
- s_id_return = id_return;
- }
-
- return id_return;
-}
-#endif /* WIN32 */
-
/*
* pg_dump and pg_restore call this to register the cleanup handler
* as soon as they've created the ArchiveHandle.
@@ -918,11 +871,6 @@ ParallelBackupStart(ArchiveHandle *AH)
pstate->parallelSlot =
pg_malloc0_array(ParallelSlot, pstate->numWorkers);
-#ifdef WIN32
- /* Make fmtId() and fmtQualifiedId() use thread-local storage */
- getLocalPQExpBuffer = getThreadLocalPQExpBuffer;
-#endif
-
/*
* Set the pstate in shutdown_info, to tell the exit handler that it must
* clean up workers as well as the main database connection. But we don't
diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c
index 7a762251f3..b3738c25db 100644
--- a/src/fe_utils/string_utils.c
+++ b/src/fe_utils/string_utils.c
@@ -34,14 +34,15 @@ static int fmtIdEncoding = -1;
* Returns a temporary PQExpBuffer, valid until the next call to the function.
* This is used by fmtId and fmtQualifiedId.
*
- * Non-reentrant and non-thread-safe but reduces memory leakage. You can
- * replace this with a custom version by setting the getLocalPQExpBuffer
+ * The buffer is thread-local, so this is safe to call from multiple threads
+ * at once; each thread gets its own. Not reentrant within a thread, but it
+ * reduces memory leakage. Replace it by setting the getLocalPQExpBuffer
* function pointer.
*/
static PQExpBuffer
defaultGetLocalPQExpBuffer(void)
{
- static PQExpBuffer id_return = NULL;
+ static _Thread_local PQExpBuffer id_return = NULL;
if (id_return) /* first time through? */
{
--
2.54.0.windows.1