From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: =?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org>,
        Euler Taveira <euler@eulerto.com>,
        "ranier.vf@gmail.com" <ranier.vf@gmail.com>,
        pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Small memory fixes for pg_createsubcriber
In-reply-to: <3357824.1739381901@sss.pgh.pa.us>
References: <CAEudQAp=AW5dJXrGLbC_aZg_9nOo=42W7uLDRONFQE-gcgnkgQ@mail.gmail.com> <68d6d810-4ef1-4f62-b47f-89b21501249b@app.fastmail.com> <obqvpv4yx57lbnfiqvy4pmnfi2l5wn6xzoq2ftytxa426pa226@2sc46frt5xtb> <3337507.1739376124@sss.pgh.pa.us> <xseczrku4nm3p4rujapxiiuttpqrpagkbwrr2nlbnmwemh525y@3he54emhpqha> <878qqb2ibm.fsf@wibble.ilmari.org> <3355447.1739380997@sss.pgh.pa.us> <875xlf2hre.fsf@wibble.ilmari.org> <3357824.1739381901@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Wed, 12 Feb 2025 12:38:21 -0500"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <3364233.1739385115.0@sss.pgh.pa.us>
Date: Wed, 12 Feb 2025 13:35:20 -0500
Message-ID: <3364495.1739385320@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/3364495.1739385320%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3364233.1739385115.1@sss.pgh.pa.us>

I experimented with the other approach: hack libpq.so to depend on
dmalloc, leaving the rest of the system alone, so that libpq's
allocations could not be freed elsewhere nor vice versa.

It could not even get through initdb, crashing here:

replace_guc_value(char **lines, const char *guc_name, const char *guc_value,
                  bool mark_as_comment)
{
    PQExpBuffer newline = createPQExpBuffer();

    ...
    free(newline);            /* but don't free newline->data */

which upon investigation is expected, because createPQExpBuffer is
exported by libpq and therefore is returning space allocated within
the shlib.  Diking out that particular free() call just allows it
to crash a bit later on, because initdb is totally full of direct
manipulations of PQExpBuffer contents.

This indicates to me that we have a *far* larger problem than
we thought, if we'd like to be totally clean on this point.

Realistically, it's not going to happen that way; it's just
not worth the trouble and notational mess.  I think if we're
honest we should just document that such-and-such combinations
of libpq and our client programs will work on Windows.

For amusement's sake, totally dirty hack-and-slash patch attached.
(I tested this on macOS, with dmalloc from MacPorts; adjust SHLIB_LINK
to suit on other platforms.)

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="use-dmalloc-within-libpq-hack-hack-hack.patch";
	charset="us-ascii"
Content-ID: <3364233.1739385115.2@sss.pgh.pa.us>
Content-Description: use-dmalloc-within-libpq-hack-hack-hack.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 701810a272a..91dc2e14d99 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -81,7 +81,7 @@ endif
 # that are built correctly for use in a shlib.
 SHLIB_LINK_INTERNAL =3D -lpgcommon_shlib -lpgport_shlib
 ifneq ($(PORTNAME), win32)
-SHLIB_LINK +=3D $(filter -lcrypt -ldes -lcom_err -lcrypto -lk5crypto -lkr=
b5 -lgssapi_krb5 -lgss -lgssapi -lssl -lsocket -lnsl -lresolv -lintl -lm, =
$(LIBS)) $(LDAP_LIBS_FE) $(PTHREAD_LIBS)
+SHLIB_LINK +=3D $(filter -lcrypt -ldes -lcom_err -lcrypto -lk5crypto -lkr=
b5 -lgssapi_krb5 -lgss -lgssapi -lssl -lsocket -lnsl -lresolv -lintl -lm, =
$(LIBS)) $(LDAP_LIBS_FE) -L/opt/local/lib -ldmalloc $(PTHREAD_LIBS)
 else
 SHLIB_LINK +=3D $(filter -lcrypt -ldes -lcom_err -lcrypto -lk5crypto -lkr=
b5 -lgssapi32 -lssl -lsocket -lnsl -lresolv -lintl -lm $(PTHREAD_LIBS), $(=
LIBS)) $(LDAP_LIBS_FE)
 endif
@@ -116,11 +116,6 @@ backend_src =3D $(top_srcdir)/src/backend
 # coding rule.
 libpq-refs-stamp: $(shlib)
 ifneq ($(enable_coverage), yes)
-ifeq (,$(filter solaris,$(PORTNAME)))
-	@if nm -A -u $< 2>/dev/null | grep -v -e __cxa_atexit -e __tsan_func_exi=
t | grep exit; then \
-		echo 'libpq must not be calling any function which invokes exit'; exit =
1; \
-	fi
-endif
 endif
 	touch $@
 =

diff --git a/src/interfaces/libpq/fe-auth-sasl.h b/src/interfaces/libpq/fe=
-auth-sasl.h
index f06f547c07d..1abafe8d34b 100644
--- a/src/interfaces/libpq/fe-auth-sasl.h
+++ b/src/interfaces/libpq/fe-auth-sasl.h
@@ -146,7 +146,7 @@ typedef struct pg_fe_sasl_mech
 	 *   state:    The opaque mechanism state returned by init()
 	 *--------
 	 */
-	void		(*free) (void *state);
+	void		(*saslfree) (void *state);
 =

 } pg_fe_sasl_mech;
 =

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-c=
onnect.c
index 85d1ca2864f..56ff7bc3b9d 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -573,7 +573,7 @@ pqDropConnection(PGconn *conn, bool flushInput)
 #endif
 	if (conn->sasl_state)
 	{
-		conn->sasl->free(conn->sasl_state);
+		conn->sasl->saslfree(conn->sasl_state);
 		conn->sasl_state =3D NULL;
 	}
 }
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq=
-int.h
index 2546f9f8a50..7296d00b34f 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -20,6 +20,8 @@
 #ifndef LIBPQ_INT_H
 #define LIBPQ_INT_H
 =

+#include <dmalloc.h>
+
 /* We assume libpq-fe.h has already been included. */
 #include "libpq-events.h"
 =


------- =_aaaaaaaaaa0--