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
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 |