From 70bef494000e4dbbeca0f0a40347ca1747aea701 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 24 Oct 2021 12:38:26 -0400 Subject: [PATCH] Fix minor memory leaks in pg_dump. I found these by running pg_dump under "valgrind --leak-check=full". The changes in flagInhIndexes() and getIndexes() replace allocation of an array of which we use only some elements by individual allocations of just the actually-needed objects. The previous coding wasted some memory, but more importantly it confused valgrind's leak tracking. collectComments() and collectSecLabels() remain major blots on the valgrind report, because they don't PQclear their query results, in order to avoid a lot of strdup's. That's a dubious tradeoff, but I'll leave it alone here; an upcoming patch will modify those functions enough to justify changing the tradeoff. --- src/bin/pg_dump/common.c | 42 +++++++++---------- src/bin/pg_dump/pg_backup_archiver.c | 2 + src/bin/pg_dump/pg_dump.c | 63 +++++++++++++++------------- 3 files changed, 56 insertions(+), 51 deletions(-) diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c index 0170fe036f..e38ff3c030 100644 --- a/src/bin/pg_dump/common.c +++ b/src/bin/pg_dump/common.c @@ -260,6 +260,8 @@ getSchemaData(Archive *fout, int *numTablesPtr) pg_log_info("reading subscriptions"); getSubscriptions(fout); + free(inhinfo); /* not needed any longer */ + *numTablesPtr = numTables; return tblinfo; } @@ -373,24 +375,20 @@ static void flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables) { int i, - j, - k; + j; for (i = 0; i < numTables; i++) { - IndexAttachInfo *attachinfo; - if (!tblinfo[i].ispartition || tblinfo[i].numParents == 0) continue; Assert(tblinfo[i].numParents == 1); - attachinfo = (IndexAttachInfo *) - pg_malloc0(tblinfo[i].numIndexes * sizeof(IndexAttachInfo)); - for (j = 0, k = 0; j < tblinfo[i].numIndexes; j++) + for (j = 0; j < tblinfo[i].numIndexes; j++) { IndxInfo *index = &(tblinfo[i].indexes[j]); IndxInfo *parentidx; + IndexAttachInfo *attachinfo; if (index->parentidx == 0) continue; @@ -399,14 +397,16 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables) if (parentidx == NULL) continue; - attachinfo[k].dobj.objType = DO_INDEX_ATTACH; - attachinfo[k].dobj.catId.tableoid = 0; - attachinfo[k].dobj.catId.oid = 0; - AssignDumpId(&attachinfo[k].dobj); - attachinfo[k].dobj.name = pg_strdup(index->dobj.name); - attachinfo[k].dobj.namespace = index->indextable->dobj.namespace; - attachinfo[k].parentIdx = parentidx; - attachinfo[k].partitionIdx = index; + attachinfo = (IndexAttachInfo *) pg_malloc(sizeof(IndexAttachInfo)); + + attachinfo->dobj.objType = DO_INDEX_ATTACH; + attachinfo->dobj.catId.tableoid = 0; + attachinfo->dobj.catId.oid = 0; + AssignDumpId(&attachinfo->dobj); + attachinfo->dobj.name = pg_strdup(index->dobj.name); + attachinfo->dobj.namespace = index->indextable->dobj.namespace; + attachinfo->parentIdx = parentidx; + attachinfo->partitionIdx = index; /* * We must state the DO_INDEX_ATTACH object's dependencies @@ -423,17 +423,15 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables) * will not try to run the ATTACH concurrently with other * operations on those tables. */ - addObjectDependency(&attachinfo[k].dobj, index->dobj.dumpId); - addObjectDependency(&attachinfo[k].dobj, parentidx->dobj.dumpId); - addObjectDependency(&attachinfo[k].dobj, + addObjectDependency(&attachinfo->dobj, index->dobj.dumpId); + addObjectDependency(&attachinfo->dobj, parentidx->dobj.dumpId); + addObjectDependency(&attachinfo->dobj, index->indextable->dobj.dumpId); - addObjectDependency(&attachinfo[k].dobj, + addObjectDependency(&attachinfo->dobj, parentidx->indextable->dobj.dumpId); /* keep track of the list of partitions in the parent index */ - simple_ptr_list_append(&parentidx->partattaches, &attachinfo[k].dobj); - - k++; + simple_ptr_list_append(&parentidx->partattaches, &attachinfo->dobj); } } } diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index ee06dc6822..9b0e699ce8 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -3374,6 +3374,8 @@ _selectTableAccessMethod(ArchiveHandle *AH, const char *tableam) destroyPQExpBuffer(cmd); + if (AH->currTableAm) + free(AH->currTableAm); AH->currTableAm = pg_strdup(want); } diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index b401b923ac..e9f68e8986 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -6869,7 +6869,6 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) PQExpBuffer query = createPQExpBuffer(); PGresult *res; IndxInfo *indxinfo; - ConstraintInfo *constrinfo; int i_tableoid, i_oid, i_indexname, @@ -7133,7 +7132,6 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) tbinfo->indexes = indxinfo = (IndxInfo *) pg_malloc(ntups * sizeof(IndxInfo)); - constrinfo = (ConstraintInfo *) pg_malloc(ntups * sizeof(ConstraintInfo)); tbinfo->numIndexes = ntups; for (j = 0; j < ntups; j++) @@ -7173,28 +7171,31 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) * If we found a constraint matching the index, create an * entry for it. */ - constrinfo[j].dobj.objType = DO_CONSTRAINT; - constrinfo[j].dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_contableoid)); - constrinfo[j].dobj.catId.oid = atooid(PQgetvalue(res, j, i_conoid)); - AssignDumpId(&constrinfo[j].dobj); - constrinfo[j].dobj.dump = tbinfo->dobj.dump; - constrinfo[j].dobj.name = pg_strdup(PQgetvalue(res, j, i_conname)); - constrinfo[j].dobj.namespace = tbinfo->dobj.namespace; - constrinfo[j].contable = tbinfo; - constrinfo[j].condomain = NULL; - constrinfo[j].contype = contype; + ConstraintInfo *constrinfo; + + constrinfo = (ConstraintInfo *) pg_malloc(sizeof(ConstraintInfo)); + constrinfo->dobj.objType = DO_CONSTRAINT; + constrinfo->dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_contableoid)); + constrinfo->dobj.catId.oid = atooid(PQgetvalue(res, j, i_conoid)); + AssignDumpId(&constrinfo->dobj); + constrinfo->dobj.dump = tbinfo->dobj.dump; + constrinfo->dobj.name = pg_strdup(PQgetvalue(res, j, i_conname)); + constrinfo->dobj.namespace = tbinfo->dobj.namespace; + constrinfo->contable = tbinfo; + constrinfo->condomain = NULL; + constrinfo->contype = contype; if (contype == 'x') - constrinfo[j].condef = pg_strdup(PQgetvalue(res, j, i_condef)); + constrinfo->condef = pg_strdup(PQgetvalue(res, j, i_condef)); else - constrinfo[j].condef = NULL; - constrinfo[j].confrelid = InvalidOid; - constrinfo[j].conindex = indxinfo[j].dobj.dumpId; - constrinfo[j].condeferrable = *(PQgetvalue(res, j, i_condeferrable)) == 't'; - constrinfo[j].condeferred = *(PQgetvalue(res, j, i_condeferred)) == 't'; - constrinfo[j].conislocal = true; - constrinfo[j].separate = true; - - indxinfo[j].indexconstraint = constrinfo[j].dobj.dumpId; + constrinfo->condef = NULL; + constrinfo->confrelid = InvalidOid; + constrinfo->conindex = indxinfo[j].dobj.dumpId; + constrinfo->condeferrable = *(PQgetvalue(res, j, i_condeferrable)) == 't'; + constrinfo->condeferred = *(PQgetvalue(res, j, i_condeferred)) == 't'; + constrinfo->conislocal = true; + constrinfo->separate = true; + + indxinfo[j].indexconstraint = constrinfo->dobj.dumpId; } else { @@ -11878,6 +11879,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo) char *funcsig; /* identity signature */ char *funcfullsig = NULL; /* full signature */ char *funcsig_tag; + char *qual_funcsig; char *proretset; char *prosrc; char *probin; @@ -12168,15 +12170,17 @@ dumpFunc(Archive *fout, const FuncInfo *finfo) funcsig_tag = format_function_signature(fout, finfo, false); + qual_funcsig = psprintf("%s.%s", + fmtId(finfo->dobj.namespace->dobj.name), + funcsig); + if (prokind[0] == PROKIND_PROCEDURE) keyword = "PROCEDURE"; else keyword = "FUNCTION"; /* works for window functions too */ - appendPQExpBuffer(delqry, "DROP %s %s.%s;\n", - keyword, - fmtId(finfo->dobj.namespace->dobj.name), - funcsig); + appendPQExpBuffer(delqry, "DROP %s %s;\n", + keyword, qual_funcsig); appendPQExpBuffer(q, "CREATE %s %s.%s", keyword, @@ -12329,9 +12333,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo) append_depends_on_extension(fout, q, &finfo->dobj, "pg_catalog.pg_proc", keyword, - psprintf("%s.%s", - fmtId(finfo->dobj.namespace->dobj.name), - funcsig)); + qual_funcsig); if (dopt->binary_upgrade) binary_upgrade_extension_member(q, &finfo->dobj, @@ -12376,6 +12378,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo) if (funcfullsig) free(funcfullsig); free(funcsig_tag); + free(qual_funcsig); if (allargtypes) free(allargtypes); if (argmodes) @@ -14768,6 +14771,8 @@ dumpForeignServer(Archive *fout, const ForeignServerInfo *srvinfo) srvinfo->rolname, srvinfo->dobj.catId, srvinfo->dobj.dumpId); + PQclear(res); + free(qsrvname); destroyPQExpBuffer(q); -- 2.39.5