Re: Small memory fixes for pg_createsubcriber - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Small memory fixes for pg_createsubcriber
Date
Msg-id 3364495.1739385320@sss.pgh.pa.us
Whole thread Raw
In response to Re: Small memory fixes for pg_createsubcriber  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
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

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 = -lpgcommon_shlib -lpgport_shlib
 ifneq ($(PORTNAME), win32)
-SHLIB_LINK += $(filter -lcrypt -ldes -lcom_err -lcrypto -lk5crypto -lkrb5 -lgssapi_krb5 -lgss -lgssapi -lssl -lsocket
-lnsl-lresolv -lintl -lm, $(LIBS)) $(LDAP_LIBS_FE) $(PTHREAD_LIBS) 
+SHLIB_LINK += $(filter -lcrypt -ldes -lcom_err -lcrypto -lk5crypto -lkrb5 -lgssapi_krb5 -lgss -lgssapi -lssl -lsocket
-lnsl-lresolv -lintl -lm, $(LIBS)) $(LDAP_LIBS_FE) -L/opt/local/lib -ldmalloc $(PTHREAD_LIBS) 
 else
 SHLIB_LINK += $(filter -lcrypt -ldes -lcom_err -lcrypto -lk5crypto -lkrb5 -lgssapi32 -lssl -lsocket -lnsl -lresolv
-lintl-lm $(PTHREAD_LIBS), $(LIBS)) $(LDAP_LIBS_FE) 
 endif
@@ -116,11 +116,6 @@ backend_src = $(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_exit | 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-connect.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 = 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"


pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: explain analyze rows=%.0f
Next
From: Álvaro Herrera
Date:
Subject: Re: pg_stat_statements and "IN" conditions