Content-Type: multipart/mixed; boundary="------------YWDNHRh0sD0Djd0furUg2nUD"
Message-ID: <178e0d90-26d9-4c3a-9448-2f64f0d35dc8@iki.fi>
Date: Fri, 27 Mar 2026 10:59:32 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Memory leak in pg_stat_statements when qtext file contains
 invalid encoding
To: Lukas Fittl <lukas@fittl.com>, Gaurav Singh <gaurav.singh@yugabyte.com>
Cc: pgsql-bugs@lists.postgresql.org
References: <CAEcQ1bYR9s4eQLFDjzzJHU8fj-MTbmRpW-9J-r2gsCn+HEsynw@mail.gmail.com>
 <CAP53Pky1575W1euSUwj2Zc5b0kBFZqpNuvMOGCvU_bR1tmD7zA@mail.gmail.com>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <CAP53Pky1575W1euSUwj2Zc5b0kBFZqpNuvMOGCvU_bR1tmD7zA@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/178e0d90-26d9-4c3a-9448-2f64f0d35dc8%40iki.fi>
Precedence: bulk

This is a multi-part message in MIME format.
--------------YWDNHRh0sD0Djd0furUg2nUD
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 27/03/2026 10:21, Lukas Fittl wrote:
> Hi Gaurav,
> 
> On Fri, Mar 27, 2026 at 12:54 AM Gaurav Singh <gaurav.singh@yugabyte.com> wrote:
>> If the qtext file contains an invalid encoding, pg_any_to_server calls ereport(ERROR) which longjmps out of the function.
>> The cleanup code at the bottom of the function is never reached.
>>
>> LWLockRelease(pgss->lock);
>> if (qbuffer)
>> free(qbuffer);
>> On every subsequent call, the malloc'd buffer (the entire file contents) is leaked, and the LWLock release is also skipped.
> 
> I don't think the analysis is correct in regards to the LWLock release
> - that should be taken care of by LWLockReleaseAll on abort.
> 
> But I think you're correct about qbuffer - because that buffer is
> using malloc (not palloc), its not part of any memory context, and so
> it will happily leak on abort.

Yep

> It appears our use of malloc in pg_stat_statements is so that we can
> fail on OOM and return NULL without a jump. I think that makes sense
> for when a GC cycle was triggered during regular query execution
> (since we don't want to error the original query), but it seems like
> just bubbling up the OOM if needed when querying the
> pg_stat_statements function seems fine.
> 
> I wonder if its worth separating the two cases, since the issue you're
> describing (the call to pg_any_to_server failing) only happens when
> returning the query text file contents to the client. I think your
> PG_FINALLY suggestion could also work, but it feels a bit tedious to
> wrap the whole pg_stat_statements_internal function in it.

Hmm, perhaps. But there's a simpler, less invasive fix. When that code 
was written, we didn't have MCXT_ALLOC_HUGE nor MCXT_ALLOC_NO_OOM. Now 
that we do, we can just use palloc_extended(MCXT_ALLOC_HUGE | 
MCXT_ALLOC_NO_OOM) instead of raw malloc(). Per attached.

- Heikki

--------------YWDNHRh0sD0Djd0furUg2nUD
Content-Type: text/x-patch; charset=UTF-8;
 name="0001-Avoid-memory-leak-on-error-while-parsing-pg_stat_sta.patch"
Content-Disposition: attachment;
 filename*0="0001-Avoid-memory-leak-on-error-while-parsing-pg_stat_sta.pa";
 filename*1="tch"
Content-Transfer-Encoding: base64

RnJvbSBhODhmZGQ3OWUwMTMxYTI1MTQxZjVlZmQ4NDI0NjAyNmIzYjdmNmZmIE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBIZWlra2kgTGlubmFrYW5nYXMgPGhlaWtraS5saW5u
YWthbmdhc0Bpa2kuZmk+CkRhdGU6IEZyaSwgMjcgTWFyIDIwMjYgMTA6NTI6MDQgKzAyMDAK
U3ViamVjdDogW1BBVENIIDEvMV0gQXZvaWQgbWVtb3J5IGxlYWsgb24gZXJyb3Igd2hpbGUg
cGFyc2luZwogcGdfc3RhdF9zdGF0ZW1lbnRzIGR1bXAgZmlsZQoKQnkgdXNpbmcgcGFsbG9j
KCkgaW5zdGVhZCBvZiByYXcgbWFsbG9jKCkuCgpSZXBvcnRlZC1ieTogR2F1cmF2IFNpbmdo
IDxnYXVyYXYuc2luZ2hAeXVnYWJ5dGUuY29tPgpSZXZpZXdlZC1ieTogTHVrYXMgRml0dGwg
PGx1a2FzQGZpdHRsLmNvbT4KRGlzY3Vzc2lvbjogaHR0cHM6Ly93d3cucG9zdGdyZXNxbC5v
cmcvbWVzc2FnZS1pZC9DQUVjUTFiWVI5czRlUUxGRGp6ekpIVThmai1NVGJtUnBXLTlKLXIy
Z3NDbitIRXN5bndAbWFpbC5nbWFpbC5jb20KLS0tCiAuLi4vcGdfc3RhdF9zdGF0ZW1lbnRz
L3BnX3N0YXRfc3RhdGVtZW50cy5jICAgfCAyMiArKysrKysrKysrKy0tLS0tLS0tCiAxIGZp
bGUgY2hhbmdlZCwgMTMgaW5zZXJ0aW9ucygrKSwgOSBkZWxldGlvbnMoLSkKCmRpZmYgLS1n
aXQgYS9jb250cmliL3BnX3N0YXRfc3RhdGVtZW50cy9wZ19zdGF0X3N0YXRlbWVudHMuYyBi
L2NvbnRyaWIvcGdfc3RhdF9zdGF0ZW1lbnRzL3BnX3N0YXRfc3RhdGVtZW50cy5jCmluZGV4
IDZjYjE0ODI0ZWMzLi43OTc1NDc2Yjg5MCAxMDA2NDQKLS0tIGEvY29udHJpYi9wZ19zdGF0
X3N0YXRlbWVudHMvcGdfc3RhdF9zdGF0ZW1lbnRzLmMKKysrIGIvY29udHJpYi9wZ19zdGF0
X3N0YXRlbWVudHMvcGdfc3RhdF9zdGF0ZW1lbnRzLmMKQEAgLTgwNSw3ICs4MDUsNyBAQCBw
Z3NzX3NobWVtX3NodXRkb3duKGludCBjb2RlLCBEYXR1bSBhcmcpCiAJaWYgKGZ3cml0ZSgm
cGdzcy0+c3RhdHMsIHNpemVvZihwZ3NzR2xvYmFsU3RhdHMpLCAxLCBmaWxlKSAhPSAxKQog
CQlnb3RvIGVycm9yOwogCi0JZnJlZShxYnVmZmVyKTsKKwlwZnJlZShxYnVmZmVyKTsKIAlx
YnVmZmVyID0gTlVMTDsKIAogCWlmIChGcmVlRmlsZShmaWxlKSkKQEAgLTgyOSw3ICs4Mjks
OCBAQCBlcnJvcjoKIAkJCShlcnJjb2RlX2Zvcl9maWxlX2FjY2VzcygpLAogCQkJIGVycm1z
ZygiY291bGQgbm90IHdyaXRlIGZpbGUgXCIlc1wiOiAlbSIsCiAJCQkJCVBHU1NfRFVNUF9G
SUxFICIudG1wIikpKTsKLQlmcmVlKHFidWZmZXIpOworCWlmIChxYnVmZmVyKQorCQlwZnJl
ZShxYnVmZmVyKTsKIAlpZiAoZmlsZSkKIAkJRnJlZUZpbGUoZmlsZSk7CiAJdW5saW5rKFBH
U1NfRFVNUF9GSUxFICIudG1wIik7CkBAIC0xODI1LDcgKzE4MjYsOCBAQCBwZ19zdGF0X3N0
YXRlbWVudHNfaW50ZXJuYWwoRnVuY3Rpb25DYWxsSW5mbyBmY2luZm8sCiAJCQlwZ3NzLT5l
eHRlbnQgIT0gZXh0ZW50IHx8CiAJCQlwZ3NzLT5nY19jb3VudCAhPSBnY19jb3VudCkKIAkJ
ewotCQkJZnJlZShxYnVmZmVyKTsKKwkJCWlmIChxYnVmZmVyKQorCQkJCXBmcmVlKHFidWZm
ZXIpOwogCQkJcWJ1ZmZlciA9IHF0ZXh0X2xvYWRfZmlsZSgmcWJ1ZmZlcl9zaXplKTsKIAkJ
fQogCX0KQEAgLTIwNDYsNyArMjA0OCw4IEBAIHBnX3N0YXRfc3RhdGVtZW50c19pbnRlcm5h
bChGdW5jdGlvbkNhbGxJbmZvIGZjaW5mbywKIAogCUxXTG9ja1JlbGVhc2UocGdzcy0+bG9j
ayk7CiAKLQlmcmVlKHFidWZmZXIpOworCWlmIChxYnVmZmVyKQorCQlwZnJlZShxYnVmZmVy
KTsKIH0KIAogLyogTnVtYmVyIG9mIG91dHB1dCBhcmd1bWVudHMgKGNvbHVtbnMpIGZvciBw
Z19zdGF0X3N0YXRlbWVudHNfaW5mbyAqLwpAQCAtMjMzMyw3ICsyMzM2LDcgQEAgZXJyb3I6
CiB9CiAKIC8qCi0gKiBSZWFkIHRoZSBleHRlcm5hbCBxdWVyeSB0ZXh0IGZpbGUgaW50byBh
IG1hbGxvYydkIGJ1ZmZlci4KKyAqIFJlYWQgdGhlIGV4dGVybmFsIHF1ZXJ5IHRleHQgZmls
ZSBpbnRvIGEgcGFsbG9jJ2QgYnVmZmVyLgogICoKICAqIFJldHVybnMgTlVMTCAod2l0aG91
dCB0aHJvd2luZyBhbiBlcnJvcikgaWYgdW5hYmxlIHRvIHJlYWQsIGVnCiAgKiBmaWxlIG5v
dCB0aGVyZSBvciBpbnN1ZmZpY2llbnQgbWVtb3J5LgpAQCAtMjM3NSw3ICsyMzc4LDcgQEAg
cXRleHRfbG9hZF9maWxlKFNpemUgKmJ1ZmZlcl9zaXplKQogCiAJLyogQWxsb2NhdGUgYnVm
ZmVyOyBiZXdhcmUgdGhhdCBvZmZfdCBtaWdodCBiZSB3aWRlciB0aGFuIHNpemVfdCAqLwog
CWlmIChzdGF0LnN0X3NpemUgPD0gTWF4QWxsb2NIdWdlU2l6ZSkKLQkJYnVmID0gKGNoYXIg
KikgbWFsbG9jKHN0YXQuc3Rfc2l6ZSk7CisJCWJ1ZiA9IChjaGFyICopIHBhbGxvY19leHRl
bmRlZChzdGF0LnN0X3NpemUsIE1DWFRfQUxMT0NfSFVHRSB8IE1DWFRfQUxMT0NfTk9fT09N
KTsKIAllbHNlCiAJCWJ1ZiA9IE5VTEw7CiAJaWYgKGJ1ZiA9PSBOVUxMKQpAQCAtMjQxNCw3
ICsyNDE3LDcgQEAgcXRleHRfbG9hZF9maWxlKFNpemUgKmJ1ZmZlcl9zaXplKQogCQkJCQkJ
KGVycmNvZGVfZm9yX2ZpbGVfYWNjZXNzKCksCiAJCQkJCQkgZXJybXNnKCJjb3VsZCBub3Qg
cmVhZCBmaWxlIFwiJXNcIjogJW0iLAogCQkJCQkJCQlQR1NTX1RFWFRfRklMRSkpKTsKLQkJ
CWZyZWUoYnVmKTsKKwkJCXBmcmVlKGJ1Zik7CiAJCQlDbG9zZVRyYW5zaWVudEZpbGUoZmQp
OwogCQkJcmV0dXJuIE5VTEw7CiAJCX0KQEAgLTI2MjUsNyArMjYyOCw3IEBAIGdjX3F0ZXh0
cyh2b2lkKQogCWVsc2UKIAkJcGdzcy0+bWVhbl9xdWVyeV9sZW4gPSBBU1NVTUVEX0xFTkdU
SF9JTklUOwogCi0JZnJlZShxYnVmZmVyKTsKKwlwZnJlZShxYnVmZmVyKTsKIAogCS8qCiAJ
ICogT0ssIGNvdW50IGEgZ2FyYmFnZSBjb2xsZWN0aW9uIGN5Y2xlLiAgKE5vdGU6IGV2ZW4g
dGhvdWdoIHdlIGhhdmUKQEAgLTI2NDIsNyArMjY0NSw4IEBAIGdjX2ZhaWw6CiAJLyogY2xl
YW4gdXAgcmVzb3VyY2VzICovCiAJaWYgKHFmaWxlKQogCQlGcmVlRmlsZShxZmlsZSk7Ci0J
ZnJlZShxYnVmZmVyKTsKKwlpZiAocWJ1ZmZlcikKKwkJcGZyZWUocWJ1ZmZlcik7CiAKIAkv
KgogCSAqIFNpbmNlIHRoZSBjb250ZW50cyBvZiB0aGUgZXh0ZXJuYWwgZmlsZSBhcmUgbm93
IHVuY2VydGFpbiwgbWFyayBhbGwKLS0gCjIuNDcuMwoK

--------------YWDNHRh0sD0Djd0furUg2nUD--