[PATHC] Fix minor memory leak in pg_basebackup
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
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 1b90057..57a1ff9 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -313,6 +313,7 @@ RetrieveWalSegSize(PGconn *conn)
if (sscanf(PQgetvalue(res, 0, 0), "%d%s", &xlog_val, xlog_unit) != 2)
{
pg_log_error("WAL segment size could not be parsed");
+ PQclear(res);
return false;
}
@@ -331,6 +332,7 @@ RetrieveWalSegSize(PGconn *conn)
"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);
+ PQclear(res);
return false;
}
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
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 1b9005722a..410116492e 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -313,9 +313,12 @@ RetrieveWalSegSize(PGconn *conn)
if (sscanf(PQgetvalue(res, 0, 0), "%d%s", &xlog_val, xlog_unit) != 2)
{
pg_log_error("WAL segment size could not be parsed");
+ PQclear(res);
return false;
}
+ PQclear(res);
+
/* set the multiplier based on unit to convert xlog_val to bytes */
if (strcmp(xlog_unit, "MB") == 0)
multiplier = 1024 * 1024;
@@ -334,7 +337,6 @@ RetrieveWalSegSize(PGconn *conn)
return false;
}
- PQclear(res);
return true;
}
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