Horton

Thread

Commits

  1. dbase_redo: Fix Valgrind-reported memory leak

  2. Fix replay of create database records on standby

  1. memory leak in dbase_redo()

    Andres Freund <andres@anarazel.de> — 2025-10-09T12:08:05Z

    Hi,

I was trying to fix an independent problem reported by skink (caused by me)
and valgrind greeted me with the following complaint during a crash-restart:

==350002== 11 bytes in 1 blocks are definitely lost in loss record 19 of 121
==350002==    at 0xFD8995: MemoryContextAlloc (mcxt.c:1250)
==350002==    by 0xFD9C38: MemoryContextStrdup (mcxt.c:1751)
==350002==    by 0xFD9C7F: pstrdup (mcxt.c:1761)
==350002==    by 0xA184BF: dbase_redo (dbcommands.c:3375)
==350002==    by 0x97BD72: ApplyWalRecord (xlogrecovery.c:2002)
==350002==    by 0x97B879: PerformWalRecovery (xlogrecovery.c:1832)
==350002==    by 0x96B377: StartupXLOG (xlog.c:5894)
==350002==    by 0xCBD29F: StartupProcessMain (startup.c:258)
==350002==    by 0xCB4B63: postmaster_child_launch (launch_backend.c:268)
==350002==    by 0xCBC369: StartChildProcess (postmaster.c:3983)
==350002==    by 0xCB86BA: PostmasterMain (postmaster.c:1396)
==350002==    by 0xB84BF5: main (main.c:231)

And I think it is right. XLOG_DBASE_CREATE_FILE_COPY is careful to
pfree(parent_path), but XLOG_DBASE_CREATE_WAL_LOG isn't.

This isn't a particularly bad leak, given the amounts of data involved and the
cost of the underlying operation, yet it still seems like somethin we ought to
fix.

Greetings,

Andres Freund

  2. Re: memory leak in dbase_redo()

    Nathan Bossart <nathandbossart@gmail.com> — 2025-10-09T14:58:02Z

    On Thu, Oct 09, 2025 at 08:08:05AM -0400, Andres Freund wrote:
> And I think it is right. XLOG_DBASE_CREATE_FILE_COPY is careful to
> pfree(parent_path), but XLOG_DBASE_CREATE_WAL_LOG isn't.

It looks like this was introduced by commit 9e4f914, which was
back-patched, but the code path in question first appears in v15.  So,
presumably something like the following needs to be back-patched that far.
I can take care of it unless someone else wants it.

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 2793fd83771..4d65e8c46c2 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -3375,6 +3375,7 @@ dbase_redo(XLogReaderState *record)
         parent_path = pstrdup(dbpath);
         get_parent_directory(parent_path);
         recovery_create_dbdir(parent_path, true);
+        pfree(parent_path);

         /* Create the database directory with the version file. */
         CreateDirAndVersionFile(dbpath, xlrec->db_id, xlrec->tablespace_id,

-- 
nathan

  3. Re: memory leak in dbase_redo()

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-10-09T16:36:58Z

    On 2025-Oct-09, Nathan Bossart wrote:

> It looks like this was introduced by commit 9e4f914, which was
> back-patched, but the code path in question first appears in v15.  So,
> presumably something like the following needs to be back-patched that far.
> I can take care of it unless someone else wants it.

Hmm, it's my bug, please let me get it done.

> diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
> index 2793fd83771..4d65e8c46c2 100644
> --- a/src/backend/commands/dbcommands.c
> +++ b/src/backend/commands/dbcommands.c
> @@ -3375,6 +3375,7 @@ dbase_redo(XLogReaderState *record)
>          parent_path = pstrdup(dbpath);
>          get_parent_directory(parent_path);
>          recovery_create_dbdir(parent_path, true);
> +        pfree(parent_path);

Yeah, this LGTM.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/

  4. Re: memory leak in dbase_redo()

    Nathan Bossart <nathandbossart@gmail.com> — 2025-10-09T20:07:06Z

    On Thu, Oct 09, 2025 at 06:36:58PM +0200, Álvaro Herrera wrote:
> On 2025-Oct-09, Nathan Bossart wrote:
>> It looks like this was introduced by commit 9e4f914, which was
>> back-patched, but the code path in question first appears in v15.  So,
>> presumably something like the following needs to be back-patched that far.
>> I can take care of it unless someone else wants it.
> 
> Hmm, it's my bug, please let me get it done.

Sounds good, thanks.

-- 
nathan

  5. Re: memory leak in dbase_redo()

    Chao Li <li.evan.chao@gmail.com> — 2025-10-09T22:41:52Z

    
> On Oct 9, 2025, at 22:58, Nathan Bossart <nathandbossart@gmail.com> wrote:
> 
> On Thu, Oct 09, 2025 at 08:08:05AM -0400, Andres Freund wrote:
>> And I think it is right. XLOG_DBASE_CREATE_FILE_COPY is careful to
>> pfree(parent_path), but XLOG_DBASE_CREATE_WAL_LOG isn't.
> 
> It looks like this was introduced by commit 9e4f914, which was
> back-patched, but the code path in question first appears in v15.  So,
> presumably something like the following needs to be back-patched that far.
> I can take care of it unless someone else wants it.
> 
> diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
> index 2793fd83771..4d65e8c46c2 100644
> --- a/src/backend/commands/dbcommands.c
> +++ b/src/backend/commands/dbcommands.c
> @@ -3375,6 +3375,7 @@ dbase_redo(XLogReaderState *record)
>         parent_path = pstrdup(dbpath);
>         get_parent_directory(parent_path);
>         recovery_create_dbdir(parent_path, true);
> +        pfree(parent_path);
> 
>         /* Create the database directory with the version file. */
>         CreateDirAndVersionFile(dbpath, xlrec->db_id, xlrec->tablespace_id,
> 

pstrdup() allocates memory from current context for dest string, so the memory it returns should be free-ed. LGTM.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  6. Re: memory leak in dbase_redo()

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-10-11T14:48:44Z

    On 2025-Oct-09, Nathan Bossart wrote:

> On Thu, Oct 09, 2025 at 06:36:58PM +0200, Álvaro Herrera wrote:
> > On 2025-Oct-09, Nathan Bossart wrote:
> >> It looks like this was introduced by commit 9e4f914, which was
> >> back-patched, but the code path in question first appears in v15.  So,
> >> presumably something like the following needs to be back-patched that far.
> >> I can take care of it unless someone else wants it.
> > 
> > Hmm, it's my bug, please let me get it done.
> 
> Sounds good, thanks.

Pushed now, thanks.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Someone said that it is at least an order of magnitude more work to do
production software than a prototype. I think he is wrong by at least
an order of magnitude."                              (Brian Kernighan)