[PATHC] Fix minor memory leak in pg_basebackup

Started by Zhang, Jiealmost 6 years ago4 messageshackers
Jump to latest
#1Zhang, Jie
zhangjie2@cn.fujitsu.com

Hi all

In some cases , PGresult is not cleared.

File: src\bin\pg_basebackup\streamutil.c

bool
RetrieveWalSegSize(PGconn *conn)
{
PGresult *res;
......
res = PQexec(conn, "SHOW wal_segment_size");
if (PQresultStatus(res) != PGRES_TUPLES_OK)
{
pg_log_error("could not send replication command \"%s\": %s",
"SHOW wal_segment_size", PQerrorMessage(conn));

PQclear(res); // *** res is cleared ***
return false;
}
......
/* fetch xlog value and unit from the result */
if (sscanf(PQgetvalue(res, 0, 0), "%d%s", &xlog_val, xlog_unit) != 2)
{
pg_log_error("WAL segment size could not be parsed");
return false; // *** res is not cleared ***
}
......
if (!IsValidWalSegSize(WalSegSz))
{
pg_log_error(ngettext("WAL segment size must be a power of two between 1 MB and 1 GB, but the remote server reported a value of %d byte",
"WAL segment size must be a power of two between 1 MB and 1 GB, but the remote server reported a value of %d bytes",
WalSegSz),
WalSegSz);
return false; ; // *** res is not cleared ***
}
......

Here is a patch.

Best Regards!

Attachments:

pg_basebackup.patchapplication/octet-stream; name=pg_basebackup.patchDownload+2-0
#2Michael Paquier
michael@paquier.xyz
In reply to: Zhang, Jie (#1)
Re: [PATHC] Fix minor memory leak in pg_basebackup

On Wed, Apr 15, 2020 at 10:06:52AM +0000, Zhang, Jie wrote:

In some cases , PGresult is not cleared.

File: src\bin\pg_basebackup\streamutil.c

bool
RetrieveWalSegSize(PGconn *conn)
{
PGresult *res;

RetrieveWalSegSize() gets called only once at the beginning of
pg_basebackup and pg_receivewal, so that's not an issue that has major
effects, still that's an issue. The first one PQclear() is needed
where you say. Now for the second one, I would just move it once the
code is done with the query result, aka after calling PQgetvalue().
What do you think? Please see the attached.
--
Michael

Attachments:

basebackup-leak.patchtext/x-diff; charset=us-asciiDownload+3-1
#3Zhang, Jie
zhangjie2@cn.fujitsu.com
In reply to: Michael Paquier (#2)
RE: [PATHC] Fix minor memory leak in pg_basebackup

Hi Michael

so much the better!

-----Original Message-----
From: Michael Paquier [mailto:michael@paquier.xyz]
Sent: Thursday, April 16, 2020 2:31 PM
To: Zhang, Jie/张 杰 <zhangjie2@cn.fujitsu.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: [PATHC] Fix minor memory leak in pg_basebackup

On Wed, Apr 15, 2020 at 10:06:52AM +0000, Zhang, Jie wrote:

In some cases , PGresult is not cleared.

File: src\bin\pg_basebackup\streamutil.c

bool
RetrieveWalSegSize(PGconn *conn)
{
PGresult *res;

RetrieveWalSegSize() gets called only once at the beginning of pg_basebackup and pg_receivewal, so that's not an issue that has major effects, still that's an issue. The first one PQclear() is needed where you say. Now for the second one, I would just move it once the code is done with the query result, aka after calling PQgetvalue().
What do you think? Please see the attached.
--
Michael

#4Michael Paquier
michael@paquier.xyz
In reply to: Zhang, Jie (#3)
Re: [PATHC] Fix minor memory leak in pg_basebackup

On Thu, Apr 16, 2020 at 10:54:09AM +0000, Zhang, Jie wrote:

So much the better!

Thanks for confirming. Fixed, then.
--
Michael