On Sun, Apr 3, 2022 at 9:52 PM Andres Freund <and...@anarazel.de> wrote: > > Hi, > > On 2022-03-29 11:55:05 -0400, Robert Haas wrote: > > I committed v6 instead. > > Coverity complains that this patch added GetDatabasePath() calls without > freeing its return value. Normally that'd be easy to dismiss, due to memory > contexts, but there's no granular resets in CreateDatabaseUsingFileCopy(). And > obviously there can be a lot of relations in one database - we shouldn't hold > onto the same path over and over again.
> The case in recovery is worse, because there we don't have a memory context to > reset afaics. Oddly enough, it sure looks like we have an existing version of > this bug in the file-copy path? Yeah, I see that the createdb() and dbase_redo() had this existing problem and with this patch we have created a few more such occurrences. The attached patch fixes it. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index df16533..ff81c48 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -218,6 +218,8 @@ CreateDatabaseUsingWalLog(Oid src_dboid, Oid dst_dboid, } list_free_deep(rnodelist); + pfree(srcpath); + pfree(dstpath); } /* @@ -628,6 +630,9 @@ CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dst_dboid, Oid src_tsid, (void) XLogInsert(RM_DBASE_ID, XLOG_DBASE_CREATE_FILE_COPY | XLR_SPECIAL_REL_UPDATE); } + + pfree(srcpath); + pfree(dstpath); } table_endscan(scan); table_close(rel, AccessShareLock); @@ -3051,6 +3056,8 @@ dbase_redo(XLogReaderState *record) * We don't need to copy subdirectories */ copydir(src_path, dst_path, false); + pfree(src_path); + pfree(dst_path); } else if (info == XLOG_DBASE_CREATE_WAL_LOG) { @@ -3063,6 +3070,7 @@ dbase_redo(XLogReaderState *record) /* Create the database directory with the version file. */ CreateDirAndVersionFile(dbpath, xlrec->db_id, xlrec->tablespace_id, true); + pfree(dbpath); } else if (info == XLOG_DBASE_DROP) {