MIME-Version: 1.0
From: vignesh C <vignesh21@gmail.com>
Date: Wed, 11 Dec 2024 09:57:38 +0530
Message-ID: <CALDaNm1hewNAsZ_e6FF52a=9drmkRJxtEPrzCB6-9mkJyeBBqA@mail.gmail.com>
Subject: Memory leak in pg_logical_slot_{get,peek}_changes
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/mixed; boundary="000000000000540e5b0628f705d5"
Archived-At: <https://www.postgresql.org/message-id/CALDaNm1hewNAsZ_e6FF52a%3D9drmkRJxtEPrzCB6-9mkJyeBBqA%40mail.gmail.com>
Precedence: bulk

--000000000000540e5b0628f705d5
Content-Type: text/plain; charset="UTF-8"

Hi,

I'm starting a new thread for one of the issues reported by Sawada-san at [1].

This is a memory leak on CacheMemoryContext when using pgoutput via SQL APIs:
/* Map must live as long as the session does. */
oldctx = MemoryContextSwitchTo(CacheMemoryContext);

entry->attrmap = build_attrmap_by_name_if_req(indesc, outdesc, false);

MemoryContextSwitchTo(oldctx);
RelationClose(ancestor);

entry->attrmap is pfree'd only when validating the RelationSyncEntry
so remains even after logical decoding API calls.

This issue can be reproduced with the following test:
-- Create table
create table t1( c1 int, c2 int, c3 int) PARTITION BY LIST (c1);
create table t1_1( c2 int, c1 int, c3 int);
ALTER TABLE t1 ATTACH PARTITION t1_1 FOR VALUES IN (0, 1, 2, 3);

-- Create publication
create publication pub1 FOR TABLE t1, t1_1 WITH
(publish_via_partition_root = true);

-- Create replication slot
SELECT 'init' FROM pg_create_logical_replication_slot('test', 'pgoutput');

-- Run the below steps between Test start and Test end many times to
see the memory usage getting increased
-- Test start
insert into t1 values( 1);
SELECT count(*) FROM pg_logical_slot_get_binary_changes('test', NULL,
NULL, 'proto_version', '4', 'publication_names', 'pub1');

-- Memory increases after each invalidation and insert
SELECT * FROM pg_backend_memory_contexts where name = 'CacheMemoryContext';
-- Test end

The attached patch resolves a memory leak by ensuring that the
attribute map is properly freed during plugin shutdown. This process
is triggered by the SQL API when the decoding context is being
released. Additionally, I am conducting a review to identify and
address any similar memory leaks that may exist elsewhere in the code.

[1] - https://www.postgresql.org/message-id/CAD21AoDkAhQVSukOfH3_reuF-j4EU0-HxMqU3dU%2BbSTxsqT14Q%40mail.gmail.com

Regards,
Vignesh

--000000000000540e5b0628f705d5
Content-Type: text/x-patch; charset="US-ASCII"; 
	name="v1-0001-Fix-memory-leak-in-pgoutput-relation-attribute-ma.patch"
Content-Disposition: attachment; 
	filename="v1-0001-Fix-memory-leak-in-pgoutput-relation-attribute-ma.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_m4je26ie0>
X-Attachment-Id: f_m4je26ie0

RnJvbSAzNmU1YzEwMTA1ZDkzNGRhMGQ1MTQ3NGIwYWQ3YzViZDkwODdlMmFhIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBWaWduZXNoIDx2aWduZXNoMjFAZ21haWwuY29tPgpEYXRlOiBX
ZWQsIDExIERlYyAyMDI0IDA4OjU3OjA2ICswNTMwClN1YmplY3Q6IFtQQVRDSCB2MV0gRml4IG1l
bW9yeSBsZWFrIGluIHBnb3V0cHV0IHJlbGF0aW9uIGF0dHJpYnV0ZSBtYXAKClRoZSBwZ291dHB1
dCBtb2R1bGUgY2FjaGVzIHJlbGF0aW9uIGF0dHJpYnV0ZSBtYXAgYW5kIGZyZWVzIGl0IHVwb24K
aW52YWxpZGF0aW9uLiAgSG93ZXZlciwgdGhpcyB3YXMgbm90IGdldHRpbmcgZnJlZWQgaW5jYXNl
IG9mIFNRTCBmdW5jdGlvbnMgbGlrZQpwZ19sb2dpY2FsX3Nsb3Rfe2dldCxwZWVrfV9jaGFuZ2Vz
KCkgd2hpY2ggd291bGQgYmxvYXQgaXRzIGNhY2hlIG1lbW9yeSB1c2FnZS4KRXZlcnkgcGdfbG9n
aWNhbF9zbG90X3tnZXQscGVla31fY2hhbmdlcygpIGNhbGwgZm9yIGNoYW5nZXMgb24gcGFydGl0
aW9uIHRhYmxlCmNyZWF0ZXMgbW9yZSBibG9hdC4gVG8gYWRkcmVzcyB0aGlzLCB0aGlzIHJlbGF0
aW9uIGF0dHJpYnV0ZSBtYXAgaXMgZnJlZWQgd2hpbGUKdGhlIG91dHB1dCBwbHVnaW4gaXMgc2h1
dGRvd24uCi0tLQogc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vcGdvdXRwdXQvcGdvdXRwdXQuYyB8
IDEwICsrKysrKysrKysKIDEgZmlsZSBjaGFuZ2VkLCAxMCBpbnNlcnRpb25zKCspCgpkaWZmIC0t
Z2l0IGEvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vcGdvdXRwdXQvcGdvdXRwdXQuYyBiL3NyYy9i
YWNrZW5kL3JlcGxpY2F0aW9uL3Bnb3V0cHV0L3Bnb3V0cHV0LmMKaW5kZXggYjUwYjNkNjJlMy4u
ZjFlZjEzZDMxMyAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vcGdvdXRwdXQv
cGdvdXRwdXQuYworKysgYi9zcmMvYmFja2VuZC9yZXBsaWNhdGlvbi9wZ291dHB1dC9wZ291dHB1
dC5jCkBAIC0xNzQ3LDYgKzE3NDcsMTYgQEAgcGdvdXRwdXRfc2h1dGRvd24oTG9naWNhbERlY29k
aW5nQ29udGV4dCAqY3R4KQogewogCWlmIChSZWxhdGlvblN5bmNDYWNoZSkKIAl7CisJCVJlbGF0
aW9uU3luY0VudHJ5ICplbnRyeTsKKwkJSEFTSF9TRVFfU1RBVFVTIHN0YXR1czsKKworCQloYXNo
X3NlcV9pbml0KCZzdGF0dXMsIFJlbGF0aW9uU3luY0NhY2hlKTsKKwkJd2hpbGUgKChlbnRyeSA9
IChSZWxhdGlvblN5bmNFbnRyeSAqKSBoYXNoX3NlcV9zZWFyY2goJnN0YXR1cykpICE9IE5VTEwp
CisJCXsKKwkJCWlmIChlbnRyeS0+YXR0cm1hcCkKKwkJCQlmcmVlX2F0dHJtYXAoZW50cnktPmF0
dHJtYXApOworCQl9CisKIAkJaGFzaF9kZXN0cm95KFJlbGF0aW9uU3luY0NhY2hlKTsKIAkJUmVs
YXRpb25TeW5jQ2FjaGUgPSBOVUxMOwogCX0KLS0gCjIuNDMuMAoK
--000000000000540e5b0628f705d5--