From: Tom Lane Date: Sun, 24 Oct 2021 16:38:26 +0000 (-0400) Subject: Fix minor memory leaks in pg_dump. X-Git-Tag: REL_15_BETA1~1313 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=70bef494000e4dbbeca0f0a40347ca1747aea701;p=thirdparty%2Fpostgresql.git 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. --- diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c index 0170fe036fa..e38ff3c030a 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 ee06dc68221..9b0e699ce86 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 b401b923ac7..e9f68e89867 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);