Subject: libpqrcv_connect() leaks PGconn

From: Andres Freund <[email-masked]>
Date: Fri, 20 Jan 2023 17:12:37 -0800

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