libpqrcv_connect() leaks PGconn

From: Andres Freund <andres(at)anarazel(dot)de>
To: pgsql-hackers(at)postgresql(dot)org
Subject: libpqrcv_connect() leaks PGconn
Date: 2023-01-21 01:12:37
Message-ID: 20230121011237.q52apbvlarfv6jm6@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

We have code like this in libpqrcv_connect():

conn = palloc0(sizeof(WalReceiverConn));
conn->streamConn = PQconnectStartParams(keys, vals,
/* expand_dbname = */ true);
if (PQstatus(conn->streamConn) == CONNECTION_BAD)
{
*err = pchomp(PQerrorMessage(conn->streamConn));
return NULL;
}

[try to establish connection]

if (PQstatus(conn->streamConn) != CONNECTION_OK)
{
*err = pchomp(PQerrorMessage(conn->streamConn));
return NULL;
}

Am I missing something, or are we leaking the libpq connection in case of
errors?

It doesn't matter really for walreceiver, since it will exit anyway, but we
also use libpqwalreceiver for logical replication, where it might?

Seems pretty clear that we should do a PQfinish() before returning NULL? I
lean towards thinking that this isn't worth backpatching given the current
uses of libpq, but I could easily be convinced otherwise.

Noticed while taking another look through [1].

Greetings,

Andres Freund

[1] https://www.postgresql.org/message-id/20220925232237.p6uskba2dw6fnwj2%40awork3.anarazel.de

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Matthias van de Meent 2023-01-21 01:22:12 Re: bug: copy progress reporting of backends which run multiple COPYs
Previous Message Nathan Bossart 2023-01-21 01:12:35 Re: Inconsistency in vacuum behavior