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

On 27/03/2026 10:21, Lukas Fittl wrote:
> Hi Gaurav,
>
> On Fri, Mar 27, 2026 at 12:54 AM Gaurav Singh <gaurav(dot)singh(at)yugabyte(dot)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