On 29/06/2026 16:19, Thomas Munro wrote:
Hi,

ECPG's auto_mem_destructor() doesn't seem quite right:

1.  POSIX thread-specific keys are reset to NULL before their at-exit
destructors are called, so its call to ECPGfree_auto_mem() gets NULL
from get_auto_allocs(), so nothing much happens.  It should use the
value passed to the destructor.

2.  ECPGfree_auto_mem() doesn't seem to be the right thing to do
anyway, because the user is expected to free heap objects allocated by
the library, for example where
src/interfaces/ecpg/test/thread/alloc.pgc does this:

     char **r = NULL;
     ...
     for (i = 1; i <= REPEATS; ++i)
     {
         EXEC SQL SELECT relname INTO :r FROM pg_class WHERE relname =
'pg_class';
         free(r);
         r = NULL;
     }

With a fix for only problem #1 in place, the thread-exit destructor
double-frees "r" from the final loop.  I *think* what is wanted here
is ecpg_clear_auto_mem(), to free just the list structure and not the
values themselves.  Draft patch like that attached.

It still leaks on Windows, but that's a known issue and I have a fix
for that as part of a larger refactoring of thread-related stuff, more
on that shortly.  This looked like a bug to report separately first.

Good catch!

I wonder if we should refrain from doing the "set_auto_allocs(NULL);" call from the destructor. I guess it's harmless, but it feels a bit wrong to me. Like in the attached.

Hmm, I wonder why ecpg_raise() frees auto-allocated values for all
connections just because one connection raised an error.

I didn't look at that

- Heikki
From c1ed64757d039f318a38d44d20086792ff1cfdf0 Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email protected]>
Date: Mon, 29 Jun 2026 23:22:16 +1200
Subject: [PATCH v2] ecpg: Fix auto_mem cleanup on thread exit.

The pthread key destructor registered to free memory allocated with
ecpg_auto_alloc() on thread exit had two problems:

1.  It didn't do anything, because the OS clears the thread-specific
slot (ie the value that get_auto_allocs() will return) before calling
the destructor.  We need to use the value that is passed to the
destructor as an argument.

2.  It was using ECPGfree_auto_mem(), but that would free the actual
allocated values, and not just the list structure.  The values are
supposed to be freed by user code (for example the free(r) call in
src/interfaces/ecpg/test/thread/alloc.pgc).  We need to use
ecpg_clear_auto_mem() instead, or valid .pgc code will double-free the
most recently auto-allocated objects on thread exit.

The destructor still doesn't run at all on Windows, but a proposed patch
will address that problem.

Discussion:
Reviewed-by:
---
 src/interfaces/ecpg/ecpglib/memory.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/interfaces/ecpg/ecpglib/memory.c b/src/interfaces/ecpg/ecpglib/memory.c
index 2112e55b6e4..b510b45b28c 100644
--- a/src/interfaces/ecpg/ecpglib/memory.c
+++ b/src/interfaces/ecpg/ecpglib/memory.c
@@ -80,11 +80,13 @@ struct auto_mem
 static pthread_key_t auto_mem_key;
 static pthread_once_t auto_mem_once = PTHREAD_ONCE_INIT;
 
+static void ecpg_clear_auto_mem_list(struct auto_mem *am);
+
 static void
 auto_mem_destructor(void *arg)
 {
-	(void) arg;					/* keep the compiler quiet */
-	ECPGfree_auto_mem();
+	if (arg)
+		ecpg_clear_auto_mem_list((struct auto_mem *) arg);
 }
 
 static void
@@ -161,16 +163,22 @@ ecpg_clear_auto_mem(void)
 {
 	struct auto_mem *am = get_auto_allocs();
 
-	/* only free our own structure */
 	if (am)
 	{
-		do
-		{
-			struct auto_mem *act = am;
-
-			am = am->next;
-			ecpg_free(act);
-		} while (am);
+		ecpg_clear_auto_mem_list(am);
 		set_auto_allocs(NULL);
 	}
 }
+
+static void
+ecpg_clear_auto_mem_list(struct auto_mem *am)
+{
+	/* only free our own structure */
+	do
+	{
+		struct auto_mem *act = am;
+
+		am = am->next;
+		ecpg_free(act);
+	} while (am);
+}
-- 
2.47.3

Reply via email to