Horton

Thread

Commits

  1. Avoid memory leak on error while parsing pg_stat_statements dump file

  1. Memory leak in pg_stat_statements when qtext file contains invalid encoding

    Gaurav Singh <gaurav.singh@yugabyte.com> — 2026-03-27T07:54:11Z

    I've found a memory leak in contrib/pg_stat_statements that occurs when the
query text file (pgss_query_texts.stat) contains an invalid byte sequence.
Each call to pg_stat_statements leaks the entire malloc'd file buffer and
 fails to release the LWLock.PostgreSQL version: Discovered against PG 15.12,
verified also present in PG 18 (HEAD as of 2026-03-26). The code path is
 unchanged between versions.Platform: macOS 15.7.3 (aarch64).Although the
cause of corruption inpgss_query_texts.statis unknown, we have seen this
twice. Similar cases were found online.Eg:ERROR: invalid byte sequence for
encoding "UTF8": 0x00 in pg_stat_statements - Database Administrators Stack
Exchange
<https://dba.stackexchange.com/questions/306293/error-invalid-byte-sequence-for-encoding-utf8-0x00-in-pg-stat-statements#:~:text=Most%20of%20the%20existing%20advice,and%20replace%20but%20no%20luck>
.How to reproduce:
-- 1. Enable pg_stat_statements
CREATE EXTENSION IF NOT EXISTS pg_stat_statements;
SELECT pg_stat_statements_reset();
-- 2. Populate with many unique queries to make the qtext file large.
-- Each CTE name is padded to 63 chars (NAMEDATALEN - 1) and repeated
-- in self-joins so the normalized text is ~300 bytes per entry.
-- Run this for i = 1..2000:
WITH q_1___________________________________________ AS (SELECT 1)
SELECT * FROM q_1___________________________________________ a1,
q_1___________________________________________ a2,
q_1___________________________________________ a3;
-- (repeat with q_2, q_3, ... q_2000)
# 3. Corrupt the qtext file with a null byte.
DATA_DIR=$(psql -t -A -c "SHOW data_directory;")
printf '\x00' | dd of="$DATA_DIR/pg_stat_tmp/pgss_query_texts.stat" \
bs=1 seek=500 count=1 conv=notrunc 2>/dev/null
-- 4. Verify corruption causes the expected error:
SELECT count(*) FROM pg_stat_statements;
-- ERROR: invalid byte sequence for encoding "UTF8": 0x00
# 5. In a single psql session, run the query 2000 times and monitor
# the backend's RSS:
BACKEND_PID=$(psql -t -A -c "SELECT pg_backend_pid();")
for i in $(seq 1 2000); do
psql -c "SELECT count(*) FROM pg_stat_statements;" 2>/dev/null
done &
# Monitor in another terminal:
watch -n 2 "ps -o rss= -p $BACKEND_PID"
Output I got:RSS grows linearly with each failing query. With a ~600 KB qtext
file and 2000 iterations, the backend's RSS grew by approximately 1.2 GB:
Time(s) RSS (KB) RSS (MB)
0s 68864 67
4s 95712 93
8s 156736 153
12s 232256 226
...
38s 756800 739
42s 907264 885
46s 1052864 1028
50s 1193024 1164
54s 1281280 1251
Leak per error is approximately equal to the qtext file size (~600 KB),
confirming the file buffer is leaked on every call.
Output I expected:RSS should remain approximately constant. Each call should
either succeed or fail cleanly without leaking memory. The LWLock should
always be released.Root cause:

   - In pg_stat_statements_internal(), the function acquires pgss->lock and
    may malloc a file buffer via qtext_load_file().
   - Later, pg_any_to_server() is called inside the hash iteration loop.
   - 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.Proposed fix:Wrap the
hash iteration
loop in PG_TRY/PG_FINALLY so that the lock release and buffer free happen
even on the error path:
PG_TRY();
{
hash_seq_init(&hash_seq, pgss_hash);
while ((entry = hash_seq_search(&hash_seq)) != NULL)
{
/* ... existing loop body unchanged ... */
}
}
PG_FINALLY();
{
LWLockRelease(pgss->lock);
if (qbuffer)
free(qbuffer);
}
PG_END_TRY();

Gaurav Singh

  2. Re: Memory leak in pg_stat_statements when qtext file contains invalid encoding

    Lukas Fittl <lukas@fittl.com> — 2026-03-27T08:21:39Z

    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.

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.

Thanks,
Lukas

PS: I would recommend reviewing the use of a text format email client
for posting to the Postgres mailing lists, or significantly reducing
your formatting when sending HTML emails - your email has a lot of
styling that is hard to read (even for me in Gmail), and even harder
to quote in a plain text email response.


--
Lukas Fittl

  3. Re: Memory leak in pg_stat_statements when qtext file contains invalid encoding

    Gaurav Singh <gaurav.singh@yugabyte.com> — 2026-03-27T08:49:58Z

    Hi Lukas,

Thank you for the correction on the LWLock. You are right, LWLockReleaseAll
on abort handles that. The leak is limited to the malloc'd qbuffer.

I thought about switching to palloc for the pg_stat_statements_internal
path, but I think it would change the existing OOM behavior in a way that
upstream may not want.

Currently, when qtext_load_file fails on OOM, it returns NULL and the
function continues gracefully, returning rows with NULL query text columns.
The user still gets their result set. With palloc, an OOM
would instead throw a hard ERROR, which changes the semantics from graceful
degradation to a failure.

Additionally, qtext_load_file is called from gc_qtexts (where an ERROR
during garbage collection would abort the user's actual in-flight query)
and from pgss_shmem_shutdown (where an ERROR could interfere with a clean
server stop). Creating a separate palloc-based variant just for
pg_stat_statements_internal would avoid those issues, but it would
still change the OOM behavior from silent degradation to a
visible error for that path.

The PG_TRY/PG_FINALLY approach preserves the existing malloc-based OOM
semantics exactly as they are today. The only thing it adds is
cleanup of the malloc'd buffer when pg_any_to_server throws an encoding
error. In terms of scope, it does not need to wrap the entire function. It
only needs to cover the section after LWLockAcquire where qbuffer is
live through the end of the hash iteration loop, which is where
pg_any_to_server can throw.

I can also scope the PG_FINALLY to just free(qbuffer) since
you confirmed LWLockReleaseAll already handles the lock on abort. That
would make it even more targeted. Happy to send a patch either way.

Apologies for the HTML formatting on the previous email.
I will use plain text going forward.


Thanks,

Gaurav

On Fri, Mar 27, 2026 at 1:52 PM Lukas Fittl <lukas@fittl.com> 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.
>
> 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.
>
> Thanks,
> Lukas
>
> PS: I would recommend reviewing the use of a text format email client
> for posting to the Postgres mailing lists, or significantly reducing
> your formatting when sending HTML emails - your email has a lot of
> styling that is hard to read (even for me in Gmail), and even harder
> to quote in a plain text email response.
>
>
> --
> Lukas Fittl
>

  4. Re: Memory leak in pg_stat_statements when qtext file contains invalid encoding

    Daniel Gustafsson <daniel@yesql.se> — 2026-03-27T08:51:37Z

    > On 27 Mar 2026, at 09:21, Lukas Fittl <lukas@fittl.com> wrote:

> 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.
> 
> 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.

We could also use palloc_extended() with MCXT_ALLOC_NO_OOM to avoid erroring
out on OOM and be able to return NULL?

--
Daniel Gustafsson

  5. Re: Memory leak in pg_stat_statements when qtext file contains invalid encoding

    Gaurav Singh <gaurav.singh@yugabyte.com> — 2026-03-27T08:57:37Z

    > We could also use palloc_extended() with MCXT_ALLOC_NO_OOM to avoid
erroring
> out on OOM and be able to return NULL?

Oh, it seems palloc_extended() would be a better fix.


--
Gaurav


On Fri, Mar 27, 2026 at 2:21 PM Daniel Gustafsson <daniel@yesql.se> wrote:

> > On 27 Mar 2026, at 09:21, Lukas Fittl <lukas@fittl.com> wrote:
>
> > 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.
> >
> > 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.
>
> We could also use palloc_extended() with MCXT_ALLOC_NO_OOM to avoid
> erroring
> out on OOM and be able to return NULL?
>
> --
> Daniel Gustafsson
>
>

  6. Re: Memory leak in pg_stat_statements when qtext file contains invalid encoding

    Heikki Linnakangas <hlinnaka@iki.fi> — 2026-03-27T08:59:32Z

    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

  7. Re: Memory leak in pg_stat_statements when qtext file contains invalid encoding

    Daniel Gustafsson <daniel@yesql.se> — 2026-03-27T09:05:18Z

    > On 27 Mar 2026, at 09:59, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

> 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.

LGTM.

--
Daniel Gustafsson

  8. Re: Memory leak in pg_stat_statements when qtext file contains invalid encoding

    Heikki Linnakangas <hlinnaka@iki.fi> — 2026-03-27T11:13:09Z

    On 27/03/2026 11:05, Daniel Gustafsson wrote:
>> On 27 Mar 2026, at 09:59, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> 
>> 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.
> 
> LGTM.

Committed, thanks!


Here's one way to reproduce the invalid encoding error without 
artificially corrupting the file:

-- Run a query with a non-latin character in it. (This needs to be run 
in UTF-8 database.)
psql postgres -c 'select g as "omega Ω col" from generate_series(1, 1) g;'

-- Create a database with latin1 encoding
psql postgres -c "create database latindb encoding 'latin1' lc_ctype='C' 
lc_collate='C' template template0"

-- check pg_stat_statements() from the latin1 database
-- This fails with encoding conversion error.
PATH=~/pgsql.fsmfork/bin/ psql latindb -c "create extension 
pg_stat_statements; select * from pg_stat_statements"

If you repeat the erroring "select * from pg_stat_statements" in latindb 
many times, you can see the memory usage grow without this fix.

- Heikki

  9. Re: Memory leak in pg_stat_statements when qtext file contains invalid encoding

    Lukas Fittl <lukas@fittl.com> — 2026-03-27T15:25:47Z

    On Fri, Mar 27, 2026 at 4:13 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>
> On 27/03/2026 11:05, Daniel Gustafsson wrote:
> >> On 27 Mar 2026, at 09:59, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> >
> >> 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.
> >
> > LGTM.
>
> Committed, thanks!

Thanks for the quick fix!

TIL about MCXT_ALLOC_NO_OOM, that's useful to know about.

Thanks,
Lukas

-- 
Lukas Fittl