memory leak in pg_stat_monitor

[surajkharage19]

Description

Hi,

While working on another issue, identified a memory leak in pg_stat_monitor specifically occurring when commands are invoked via the SPI infrastructure within a Background Worker.

The root cause appears to be in the cleanup callback registration. https://github.com/percona/pg_stat_monitor/blob/main/pg_stat_monitor.c#L398
The current logic only registers the cleanup callback if MessageContext is valid. However, when executing via SPI in a Background Worker, MessageContext is typically unavailable.
Consequently, the cleanup function is never triggered, leading to a memory leak.

Attaching the sample contrib which can be used to identify the issue, just need to install the module and add entry in shared_preload_library.

Can you please check this?

spi_sample.patch

Expected Results

N/A

Actual Results

N/A

Version

Postgres 18, pg_stat_monitor 2.3

Steps to reproduce

No response

Relevant logs

Code of Conduct

• I agree to follow Percona Community Code of Conduct

[surajkharage19]

@dutow @artemgavrilov

Since MessageContext is not available when query executed through SPI from background worker, how about resetting the memory context at the end of pgsm_ExecutorEnd() and pgsm_ProcessUtility() functions?
pgsm_delete_entry() is getting called at the end of pgsm_ExecutorEnd() and pgsm_ProcessUtility() functions which clears the query from local memory context, can we reset the local memory context after that?

I would appreciate your response on this.

[surajkharage19]

Hi,

As mentioned in earlier update, memory context reset can be done at 2 places to cover DML and DDL/utility queries.

1. ExecutorEnd hook
2. ProcessUtility hook

pg_stat_monitor has implemented several hooks. To verify, I have added ELOGs at the start of each hook and observed below -

DML queries follow this route -

postgres@4114353=#select * from bar;
WARNING:  pgsm_post_parse_analyze_internal
WARNING:  pgsm_planner_hook
WARNING:  pgsm_ExecutorStart
WARNING:  pgsm_ExecutorRun
WARNING:  pgsm_ExecutorFinish
WARNING:  pgsm_ExecutorEnd
 id 
----
(0 rows)

In both examples, ExecutorEnd is the last hook that is getting called.

DDL/utility queries follow this route -

postgres@4114353=#create table bar(id int);
WARNING:  pgsm_post_parse_analyze_internal
WARNING:  pgsm_ProcessUtility
CREATE TABLE
postgres@4114353=#analyze ;
WARNING:  pgsm_post_parse_analyze_internal
WARNING:  pgsm_ProcessUtility
ANALYZE

In this case, ProcessUtility is the last hook that is getting called.

So, I have removed the existing reset logic and added the reset logic at the end in pgsm_ExecutorEnd() and pgsm_ProcessUtility() and it seems it is clearing the memory correctly.
I have ran the regression (make installcheck) and no such issues found. (there are few regression failures without my changes as well).

I also checked through SPI module that we have written previously using background worker and that is also cleaning up the local memory context similar to queries running via psql.

[AndersAstrand]

Hello!

Thank you for your bug report! I am not surprised this went unnoticed because running SPI queries from a background worker seems like a quite niche usecase 🙂. Did you perhaps run into this while using pg_cron?

We're currently doing some rework of pg_stat_monitor, and findings like this is very valuable in that work. Thank you again!

I've added a your bug report to our bug tracker here: https://perconadev.atlassian.net/browse/PG-2229

[surajkharage19]

Thanks @AndersAstrand for the acknowlegement.

Did you perhaps run into this while using pg_cron?

Not specifically. Another internal team encountered this issue during their own application development.

Does the analysis and the proposed fix I shared earlier seem accurate to you?