From: Chao Li <li.evan.chao@gmail.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_32812FDA-1F35-450E-A02E-0862971F05AC"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.300.41.1.7\))
Subject: pg_upgrade: fix memory leak in SLRU I/O code
Message-Id: <5DED17B4-D948-4C0B-9DE1-A915D0BFAA54@gmail.com>
Date: Wed, 4 Feb 2026 17:20:49 +0800
To: PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Archived-At: <https://www.postgresql.org/message-id/5DED17B4-D948-4C0B-9DE1-A915D0BFAA54%40gmail.com>
Precedence: bulk


--Apple-Mail=_32812FDA-1F35-450E-A02E-0862971F05AC
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8

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 !=3D -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=E2=80=99t see a reason to free an object itself without also freeing =
the memory owned by its members.

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/


--Apple-Mail=_32812FDA-1F35-450E-A02E-0862971F05AC
Content-Disposition: attachment;
	filename=v1-0001-pg_upgrade-fix-memory-leak-in-SLRU-I-O-code.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v1-0001-pg_upgrade-fix-memory-leak-in-SLRU-I-O-code.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=20f06d718e119cbf17ac9a7aecb41eed385a4f4f41=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20"Chao=20Li=20(Evan)"=20<lic@highgo.com>=0A=
Date:=20Wed,=204=20Feb=202026=2016:53:00=20+0800=0ASubject:=20[PATCH=20=
v1]=20pg_upgrade:=20fix=20memory=20leak=20in=20SLRU=20I/O=20code=0A=0A=
SlruSegState->dir=20is=20allocated=20in=20AllocSlruSegState()=20but=20=
was=20never=20freed.=0AAdd=20a=20common=20cleanup=20helper=20to=20=
release=20all=20resources=20associated=20with=20an=0ASLRU=20segment=20=
state=20and=20use=20it=20for=20both=20readers=20and=20writers.=0A=0A=
Author:=20Chao=20Li=20<lic@highgo.com>=0AReviewed-by:=0ADiscussion:=20=
https://postgr.es/m/=0A---=0A=20src/bin/pg_upgrade/slru_io.c=20|=2022=20=
+++++++++++++++-------=0A=201=20file=20changed,=2015=20insertions(+),=20=
7=20deletions(-)=0A=0Adiff=20--git=20a/src/bin/pg_upgrade/slru_io.c=20=
b/src/bin/pg_upgrade/slru_io.c=0Aindex=20ae3e224d7b1..efc0fb71dda=20=
100644=0A---=20a/src/bin/pg_upgrade/slru_io.c=0A+++=20=
b/src/bin/pg_upgrade/slru_io.c=0A@@=20-19,6=20+19,7=20@@=0A=20#include=20=
"slru_io.h"=0A=20=0A=20static=20SlruSegState=20*AllocSlruSegState(const=20=
char=20*dir);=0A+static=20void=20FreeSlruSegState(SlruSegState=20=
*state);=0A=20static=20char=20*SlruFileName(SlruSegState=20*state,=20=
int64=20segno);=0A=20static=20void=20SlruFlush(SlruSegState=20*state);=0A=
=20=0A@@=20-28,7=20+29,7=20@@=20AllocSlruSegState(const=20char=20*dir)=0A=
=20{=0A=20=09SlruSegState=20*state=20=3D=20pg_malloc(sizeof(*state));=0A=20=
=0A-=09state->dir=20=3D=20pstrdup(dir);=0A+=09state->dir=20=3D=20=
pg_strdup(dir);=0A=20=09state->fn=20=3D=20NULL;=0A=20=09state->fd=20=3D=20=
-1;=0A=20=09state->segno=20=3D=20-1;=0A@@=20-69,6=20+70,17=20@@=20=
AllocSlruRead(const=20char=20*dir,=20bool=20long_segment_names)=0A=20=09=
return=20state;=0A=20}=0A=20=0A+static=20void=0A=
+FreeSlruSegState(SlruSegState=20*state)=0A+{=0A+=09if=20(state->fd=20!=3D=
=20-1)=0A+=09=09close(state->fd);=0A+=0A+=09pg_free(state->fn);=0A+=09=
pg_free(state->dir);=0A+=09pg_free(state);=0A+}=0A+=0A=20/*=0A=20=20*=20=
Read=20the=20given=20page=20into=20memory=20buffer.=0A=20=20*=0A@@=20=
-154,9=20+166,7=20@@=20FreeSlruRead(SlruSegState=20*state)=0A=20{=0A=20=09=
Assert(!state->writing);=09/*=20read=20only=20mode=20*/=0A=20=0A-=09if=20=
(state->fd=20!=3D=20-1)=0A-=09=09close(state->fd);=0A-=09pg_free(state);=0A=
+=09FreeSlruSegState(state);=0A=20}=0A=20=0A=20/*=0A@@=20-263,7=20+273,5=20=
@@=20FreeSlruWrite(SlruSegState=20*state)=0A=20=0A=20=09=
SlruFlush(state);=0A=20=0A-=09if=20(state->fd=20!=3D=20-1)=0A-=09=09=
close(state->fd);=0A-=09pg_free(state);=0A+=09FreeSlruSegState(state);=0A=
=20}=0A--=20=0A2.50.1=20(Apple=20Git-155)=0A=0A=

--Apple-Mail=_32812FDA-1F35-450E-A02E-0862971F05AC--