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
Message: pg_dump: use threads for parallel workers on all platforms

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