Subject: Re: pg_upgrade: fix memory leak in SLRU I/O code

From: Yugo Nagata <[email-masked]>
Date: Thu, 5 Feb 2026 10:54:33 +0900

On Wed, 4 Feb 2026 17:20:49 +0800
Chao Li <[email-masked]> wrote:

> Hi Hackers,
> 
> This comes from a previous review and has been on my to-do list for a while.
> 
> Since src/bin/pg_upgrade/slru_io.c includes postgres_fe.h, it is frontend code, so backend memory contexts are not used here.
> 
> In the current code:
> ```
> void
> FreeSlruWrite(SlruSegState *state)
> {
> 	Assert(state->writing);
> 
> 	SlruFlush(state);
> 
> 	if (state->fd != -1)
> 		close(state->fd);
> 	pg_free(state);
> }
> ```
> 
> the SlruSegState itself is freed, but state->dir and state->fn are not, which results in a memory leak during pg_upgrade runs. More generally, I don’t see a reason to free an object itself without also freeing the memory owned by its members.

As far as I know, AllocSlruSegState() and FreeSlruWrite() are not called
repeatedly in a loop, and the amount of memory involved is small, so the
impact of the leak seems limited. That said, I agree that it is better to
also free the memory owned by the struct members.

Regards,
Yugo Nagata

> 
> While looking at this, I also noticed that state->dir is allocated using pstrdup(). To better align with frontend conventions, the patch switches this to pg_strdup() and introduces a common cleanup helper to free all resources associated with SlruSegState.
> 
> See the attached patch.
> 
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
> 
> 
> 
> 

-- 
Yugo Nagata <[email-masked]>

---

From: Chao Li <[email-masked]>
Date: Thu, 5 Feb 2026 12:02:51 +0800

> On Feb 5, 2026, at 09:54, Yugo Nagata <[email-masked]> wrote:
> 

Hi Yugo-san,

Thank you very much for the review. Yes, the leak itself is tiny, the main point for me is that the memory owned by the struct members is not freed, which can be a bit confusing for code readers. As for the change from pstrdup() to pg_strdup(), that felt too trivial to justify a separate patch, so I took this as an opportunity to clean it up at the same time.

BTW, this is the CF entry: https://commitfest.postgresql.org/patch/6462/, you may mark yourself as a reviewer.

Best regards,