Feedback-ID: i0fe9450f:Fastmail
Date: Mon, 18 Nov 2024 12:43:27 +0900
From: Michael Paquier <michael@paquier.xyz>
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Cc: by Yang <mobile.yang@outlook.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: memory leak in pgoutput
Message-ID: <Zzq334VG-l2vCAw5@paquier.xyz>
References: <DM3PR84MB3442E14B340E553313B5C816E3252@DM3PR84MB3442.NAMPRD84.PROD.OUTLOOK.COM>
 <OS0PR01MB5716606345806CA8F43B5EF794272@OS0PR01MB5716.jpnprd01.prod.outlook.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="RjN34q9EIQl7AEp9"
Content-Disposition: inline
In-Reply-To: <OS0PR01MB5716606345806CA8F43B5EF794272@OS0PR01MB5716.jpnprd01.prod.outlook.com>
Archived-At: <https://www.postgresql.org/message-id/Zzq334VG-l2vCAw5%40paquier.xyz>
Precedence: bulk


--RjN34q9EIQl7AEp9
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Nov 18, 2024 at 02:53:52AM +0000, Zhijie Hou (Fujitsu) wrote:
> But I think there is an issue in the attached patch:
>=20
> +			FreeTupleDesc(entry->old_slot->tts_tupleDescriptor);
>  			ExecDropSingleTupleTableSlot(entry->old_slot);
>=20
> Here, after freeing the tupledesc, the ExecDropSingleTupleTableSlot will =
still
> access the freed tupledesc->tdrefcount which is an illegal memory access.
>=20
> I think we can do something like below instead:
>=20
> +			TupleDesc	desc =3D entry->old_slot->tts_tupleDescriptor;
> +
> +			Assert(desc->tdrefcount =3D=3D -1);
> +
>  			ExecDropSingleTupleTableSlot(entry->old_slot);
> +			FreeTupleDesc(desc);

Yep, obviously.

I was first surprised that the DecrTupleDescRefCount() done in
ExecDropSingleTupleTableSlot() would not be enough to free the
TupleDesc.

We do some allocations for dynamically-allocated resources like what
pgoutput does in the SRF code, if I recall correctly, as these need
are a problem across multiple calls and the query-level memory context
would not do this cleanup..
--
Michael

--RjN34q9EIQl7AEp9
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmc6t98ACgkQnvQgOdby
QH0pyQ//e7Hyg78usE7U0faT49A7iZc1/4Abj17jnixffDRc/lTsgSPmnF6OrchS
FQyqU+1MmTaE5XkJF51ote9IIik+wY+IKiOa30Fb1huMwwC/X25EkJF23EKbNDtb
Lc2dMTwdqdId9dtkzXZPXHIFIWdzSbNEknItNFRa5cLwXHdwWVpWOy4P0Mr/aa6x
CQogaNiAs/WyY1LPI30h5rNE8f9W3KHSSs4S6szxjmfe0kzYm1/wAVztY5jLKaCz
K23kbJgw8RNwCeBy6TeglXUo9k9UVJJfYu9wSw19y04j9rcosnF/5YZNCUSpIC2e
i31EAeVlozK9spOalbN2oM697RHXyE6G2MLLeUQQbQyvgs7yUKX4mlQE4mNfhZuv
sbb3GXIIncsflP2NiVz/78btfhTXeGjoteiTqOWT1Q9NrQpHXqGcoueNaTXBvWhg
JF455BzZ44+iAKNnmmw4QP+jTPu9qRJKKU3hyfLY5eM2jZDMOoHBzff4y0c0U9GL
HHTH4A9wNE3j9GSzo2hb/pW4yRKhSUGuF59z6ujvgwUt1uLwxs78g3KLWMPeMNzU
0p30a29h5BCTP1xXdMEAwojuVSmG0yO926NYIyyFtjhIyk8oeSx83JdEKb66CZ5q
+12M0ypicWDGYkhu+ikEylIEobDmRPjBwVYhSTrTEy/ebzmBFz4=
=qdQT
-----END PGP SIGNATURE-----

--RjN34q9EIQl7AEp9--