1
0
mirror of https://github.com/postgres/postgres.git synced 2025-10-31 10:30:33 +03:00

Fix memory leaks in scripts/vacuuming.c.

Coverity complained that the list of table names returned by
retrieve_objects() was leaked, and it's right.  Potentially this
is quite a lot of memory; however, it's not entirely clear how much
we gain by cleaning it up, since in many operating modes we're going
to need the list until the program is about to exit.  Still, it will
win in some cases, so fix the leak.

vacuuming.c is new in v19, and this patch doesn't apply at all cleanly
to the predecessor code in v18.  I'm not excited enough about the
issue to devise a back-patch.
This commit is contained in:
Tom Lane
2025-10-22 15:19:19 -04:00
parent 9224c30252
commit bc310c6ff3

View File

@@ -40,6 +40,7 @@ static SimpleStringList *retrieve_objects(PGconn *conn,
vacuumingOptions *vacopts, vacuumingOptions *vacopts,
SimpleStringList *objects, SimpleStringList *objects,
bool echo); bool echo);
static void free_retrieved_objects(SimpleStringList *list);
static void prepare_vacuum_command(PGconn *conn, PQExpBuffer sql, static void prepare_vacuum_command(PGconn *conn, PQExpBuffer sql,
vacuumingOptions *vacopts, const char *table); vacuumingOptions *vacopts, const char *table);
static void run_vacuum_command(PGconn *conn, const char *sql, bool echo, static void run_vacuum_command(PGconn *conn, const char *sql, bool echo,
@@ -101,9 +102,13 @@ vacuuming_main(ConnParams *cparams, const char *dbname,
concurrentCons, concurrentCons,
progname, echo, quiet); progname, echo, quiet);
if (ret != 0) if (ret != 0)
{
free_retrieved_objects(found_objs);
return ret; return ret;
} }
}
free_retrieved_objects(found_objs);
return EXIT_SUCCESS; return EXIT_SUCCESS;
} }
else else
@@ -171,6 +176,7 @@ vacuum_one_database(ConnParams *cparams,
int ntups = 0; int ntups = 0;
const char *initcmd; const char *initcmd;
SimpleStringList *retobjs = NULL; SimpleStringList *retobjs = NULL;
bool free_retobjs = false;
int ret = EXIT_SUCCESS; int ret = EXIT_SUCCESS;
const char *stage_commands[] = { const char *stage_commands[] = {
"SET default_statistics_target=1; SET vacuum_cost_delay=0;", "SET default_statistics_target=1; SET vacuum_cost_delay=0;",
@@ -289,7 +295,8 @@ vacuum_one_database(ConnParams *cparams,
/* /*
* If the caller provided the results of a previous catalog query, just * If the caller provided the results of a previous catalog query, just
* use that. Otherwise, run the catalog query ourselves and set the * use that. Otherwise, run the catalog query ourselves and set the
* return variable if provided. * return variable if provided. (If it is, then freeing the string list
* becomes the caller's responsibility.)
*/ */
if (found_objs && *found_objs) if (found_objs && *found_objs)
retobjs = *found_objs; retobjs = *found_objs;
@@ -298,18 +305,22 @@ vacuum_one_database(ConnParams *cparams,
retobjs = retrieve_objects(conn, vacopts, objects, echo); retobjs = retrieve_objects(conn, vacopts, objects, echo);
if (found_objs) if (found_objs)
*found_objs = retobjs; *found_objs = retobjs;
else
free_retobjs = true;
} }
/* /*
* Count the number of objects in the catalog query result. If there are * Count the number of objects in the catalog query result. If there are
* none, we are done. * none, we are done.
*/ */
for (cell = retobjs ? retobjs->head : NULL; cell; cell = cell->next) for (cell = retobjs->head; cell; cell = cell->next)
ntups++; ntups++;
if (ntups == 0) if (ntups == 0)
{ {
PQfinish(conn); PQfinish(conn);
if (free_retobjs)
free_retrieved_objects(retobjs);
return EXIT_SUCCESS; return EXIT_SUCCESS;
} }
@@ -407,6 +418,8 @@ finish:
ParallelSlotsTerminate(sa); ParallelSlotsTerminate(sa);
pg_free(sa); pg_free(sa);
termPQExpBuffer(&sql); termPQExpBuffer(&sql);
if (free_retobjs)
free_retrieved_objects(retobjs);
return ret; return ret;
} }
@@ -425,13 +438,16 @@ vacuum_all_databases(ConnParams *cparams,
int concurrentCons, int concurrentCons,
const char *progname, bool echo, bool quiet) const char *progname, bool echo, bool quiet)
{ {
int ret = EXIT_SUCCESS;
PGconn *conn; PGconn *conn;
PGresult *result; PGresult *result;
int numdbs;
conn = connectMaintenanceDatabase(cparams, progname, echo); conn = connectMaintenanceDatabase(cparams, progname, echo);
result = executeQuery(conn, result = executeQuery(conn,
"SELECT datname FROM pg_database WHERE datallowconn AND datconnlimit <> -2 ORDER BY 1;", "SELECT datname FROM pg_database WHERE datallowconn AND datconnlimit <> -2 ORDER BY 1;",
echo); echo);
numdbs = PQntuples(result);
PQfinish(conn); PQfinish(conn);
if (vacopts->mode == MODE_ANALYZE_IN_STAGES) if (vacopts->mode == MODE_ANALYZE_IN_STAGES)
@@ -439,7 +455,7 @@ vacuum_all_databases(ConnParams *cparams,
SimpleStringList **found_objs = NULL; SimpleStringList **found_objs = NULL;
if (vacopts->missing_stats_only) if (vacopts->missing_stats_only)
found_objs = palloc0(PQntuples(result) * sizeof(SimpleStringList *)); found_objs = palloc0(numdbs * sizeof(SimpleStringList *));
/* /*
* When analyzing all databases in stages, we analyze them all in the * When analyzing all databases in stages, we analyze them all in the
@@ -451,10 +467,8 @@ vacuum_all_databases(ConnParams *cparams,
*/ */
for (int stage = 0; stage < ANALYZE_NUM_STAGES; stage++) for (int stage = 0; stage < ANALYZE_NUM_STAGES; stage++)
{ {
for (int i = 0; i < PQntuples(result); i++) for (int i = 0; i < numdbs; i++)
{ {
int ret;
cparams->override_dbname = PQgetvalue(result, i, 0); cparams->override_dbname = PQgetvalue(result, i, 0);
ret = vacuum_one_database(cparams, vacopts, stage, ret = vacuum_one_database(cparams, vacopts, stage,
objects, objects,
@@ -462,16 +476,23 @@ vacuum_all_databases(ConnParams *cparams,
concurrentCons, concurrentCons,
progname, echo, quiet); progname, echo, quiet);
if (ret != EXIT_SUCCESS) if (ret != EXIT_SUCCESS)
return ret; break;
} }
if (ret != EXIT_SUCCESS)
break;
}
if (vacopts->missing_stats_only)
{
for (int i = 0; i < numdbs; i++)
free_retrieved_objects(found_objs[i]);
pg_free(found_objs);
} }
} }
else else
{ {
for (int i = 0; i < PQntuples(result); i++) for (int i = 0; i < numdbs; i++)
{ {
int ret;
cparams->override_dbname = PQgetvalue(result, i, 0); cparams->override_dbname = PQgetvalue(result, i, 0);
ret = vacuum_one_database(cparams, vacopts, ret = vacuum_one_database(cparams, vacopts,
ANALYZE_NO_STAGE, ANALYZE_NO_STAGE,
@@ -480,13 +501,13 @@ vacuum_all_databases(ConnParams *cparams,
concurrentCons, concurrentCons,
progname, echo, quiet); progname, echo, quiet);
if (ret != EXIT_SUCCESS) if (ret != EXIT_SUCCESS)
return ret; break;
} }
} }
PQclear(result); PQclear(result);
return EXIT_SUCCESS; return ret;
} }
/* /*
@@ -784,6 +805,22 @@ retrieve_objects(PGconn *conn, vacuumingOptions *vacopts,
return found_objs; return found_objs;
} }
/*
* Free the results of retrieve_objects().
*
* For caller convenience, we allow the argument to be NULL,
* although retrieve_objects() will never return that.
*/
static void
free_retrieved_objects(SimpleStringList *list)
{
if (list)
{
simple_string_list_destroy(list);
pg_free(list);
}
}
/* /*
* Construct a vacuum/analyze command to run based on the given * Construct a vacuum/analyze command to run based on the given
* options, in the given string buffer, which may contain previous garbage. * options, in the given string buffer, which may contain previous garbage.