Re: Probable memory leak with ECPG and AIX

From: Noah Misch <noah(at)leadboat(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Guillaume Lelarge <guillaume(at)lelarge(dot)info>, Benoit Lobréau <benoit(dot)lobreau(at)gmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>, "Dr(dot) Michael Meskes" <michael(dot)meskes(at)credativ(dot)com>
Subject: Re: Probable memory leak with ECPG and AIX
Date: 2022-07-03 03:06:19
Message-ID: 20220703030619.GB2378460@rfd.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Jul 02, 2022 at 02:53:34PM -0400, Tom Lane wrote:
> This looks solid to me. The only nit I can find to pick is that I'd
> have added one more comment, along the lines of
>
> diff --git a/src/interfaces/ecpg/ecpglib/connect.c b/src/interfaces/ecpg/ecpglib/connect.c
> index 9f958b822c..96f99ae072 100644
> --- a/src/interfaces/ecpg/ecpglib/connect.c
> +++ b/src/interfaces/ecpg/ecpglib/connect.c
> @@ -508,6 +508,11 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
> #ifdef ENABLE_THREAD_SAFETY
> pthread_mutex_lock(&connections_mutex);
> #endif
> +
> + /*
> + * ... but first, make certain we have created ecpg_clocale. Rely on
> + * holding connections_mutex to ensure this is done by only one thread.
> + */
> #ifdef HAVE_USELOCALE
> if (!ecpg_clocale)
> {
>
> I've marked it RFC.

Thanks for reviewing. Pushed with that comment. prairiedog complains[1]:

ld: common symbols not allowed with MH_DYLIB output format with the -multi_module option
connect.o definition of common _ecpg_clocale (size 4)

I bet this would fix it:

--- a/src/interfaces/ecpg/ecpglib/connect.c
+++ b/src/interfaces/ecpg/ecpglib/connect.c
@@ -11,7 +11,7 @@
#include "sqlca.h"

#ifdef HAVE_USELOCALE
-locale_t ecpg_clocale;
+locale_t ecpg_clocale = (locale_t) 0;
#endif

#ifdef ENABLE_THREAD_SAFETY

I hear[1] adding -fno-common to compiler options would also fix that. Still,
in the absence of other opinions, I'll just add the no-op initialization.

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prairiedog&dt=2022-07-03%2001%3A14%3A19
[2] https://gcc.gnu.org/legacy-ml/gcc/2005-06/msg00378.html

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-07-03 03:37:08 Re: Probable memory leak with ECPG and AIX
Previous Message Robert Haas 2022-07-03 03:04:28 Re: replacing role-level NOINHERIT with a grant-level option