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

> On Feb 5, 2026, at 12:37, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Thu, Feb 05, 2026 at 12:02:51PM +0800, Chao Li wrote:
>> 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.
>
> Does it really matter memory-wise? Even in the case of state->fn,
> SlruReadSwitchPageSlow() and SlruWriteSwitchPageSlow() make sure to
> free it before switching to a new segment to process, and these state
> allocations are done once. Well, okay, twice as of the members *and*
> the offsets, but the logic as written is not going to bloat memory
> with a short loop execution.
>
> In short, I see nothing worth changing.
> --
> Michael

Hi Michael,

I agree that memory bloat is not really the issue here. If the code never freed the state at all, I wouldn’t have bothered with this patch.

My concern is more about the pattern: freeing the container structure while leaving its members allocated. That feels inconsistent and potentially confusing to readers. Either owning objects should be fully freed, or not freed at all, but partial freeing doesn’t seem like a great precedent. I’m not sure that’s a pattern we want to encourage in PG code.

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