Fix memory leak in 'ProxySQL_Admin::stats___mysql_global' - Closes #4976

[JavierJF]

Description

The resource leak was associated to prepared statements (sqlite3_stmt) used on 'ProxySQL_Admin::stats___mysql_global'.

Fix

Since the detection of these leaks is tricky, because the resources held by 'SQLite3' never lose their references, a helper type ('stmt_unique_ptr') and a safer variation for 'prepare_v2' have been introduced. This helper type should automatically handle the lifetime of the statements, avoiding these kind of leaks in the future. The non-safe alternative has been flagged as 'deprecated', so it can be easily identified in later refactors or slowly removed during development.

[renecannao]

Can one of the admins verify this patch?

[gemini-code-assist]

Summary of Changes

Hello @JavierJF, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a memory leak identified in the ProxySQL_Admin::stats___mysql_global function. The fix involves a significant refactoring of how SQLite prepared statements (sqlite3_stmt) are managed, transitioning from raw pointers to C++ smart pointers (std::unique_ptr) to ensure automatic and reliable resource deallocation.

Highlights

• Memory Leak Fix: Addressed a memory leak in the ProxySQL_Admin::stats___mysql_global function, specifically related to unfinalized sqlite3_stmt objects.
• Smart Pointer Introduction: Introduced stmt_unique_ptr, a std::unique_ptr specialization, to automatically manage the lifecycle of sqlite3_stmt objects, ensuring their proper deallocation and preventing future leaks.
• prepare_v2 Overload: Added a new SQLite3DB::prepare_v2 overload that returns a std::pair<int, stmt_unique_ptr>, promoting safer and more idiomatic C++ for statement handling.
• Deprecation: The original SQLite3DB::prepare_v2 method, which returned a raw sqlite3_stmt**, has been marked as deprecated to encourage migration to the new, safer alternative.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request effectively addresses a memory leak in ProxySQL_Admin::stats___mysql_global by introducing a RAII wrapper (stmt_unique_ptr) for sqlite3_stmt resources. The new prepare_v2 overload provides a much safer way to handle prepared statements, and deprecating the old version is a great step towards improving overall code safety.

My review includes a few suggestions to improve documentation and code conciseness. Overall, this is a solid improvement.

[cyberplant]

One more thing I noticed.. I left one server with this bug running but without any usage, and I still see memory usage increasing. Could it be because of PMM monitoring?

Also, getting the memory usage takes long time:

ProxySQLAdmin >select * from stats.stats_memory_metrics;
+-----------------------------------+----------------+
| Variable_Name                     | Variable_Value |
+-----------------------------------+----------------+
| SQLite3_memory_bytes              | -1508139680    |
| jemalloc_resident                 | 5133131776     |
| jemalloc_active                   | 4991012864     |
| jemalloc_allocated                | 4967472712     |
| jemalloc_mapped                   | 5293178880     |
| jemalloc_metadata                 | 141019480      |
| jemalloc_retained                 | 1739096064     |
| Auth_memory                       | 21596          |
| mysql_query_digest_memory         | 1757056320     |
| mysql_query_rules_memory          | 276            |
| pgsql_query_digest_memory         | 0              |
| pgsql_query_rules_memory          | 274            |
| prepare_statement_metadata_memory | 563810         |
| prepare_statement_backend_memory  | 338720         |
| mysql_firewall_users_table        | 0              |
| mysql_firewall_users_config       | 0              |
| mysql_firewall_rules_table        | 0              |
| mysql_firewall_rules_config       | 329            |
| stack_memory_mysql_threads        | 67108864       |
| stack_memory_admin_threads        | 92274688       |
| stack_memory_cluster_threads      | 41943040       |
+-----------------------------------+----------------+
21 rows in set (1.46 sec)

I started looking at this because of some slowness in ProxySQL. After a restart the memory usage goes down and speed returns back to normal.

If it helps you, I can test the new version on this server, at least to see if memory usage goes up without real usage, as it currently does.

[JavierJF]

Hi @cyberplant,

that's expected, since the leak is in stats_mysql_global and since (iirc) PMM constantly queries that table, even if the instance is idle, the leak is going to be exercised and the memory usage will increase. The leak should be fixed in this branch, we already performed testing to verify it, but you can of course test this branch to verify it on your own. Regarding the potential overflow, I will also take care of it soon in this branch.

Thanks, regards,
Javier.

[sonarqubecloud]

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[cyberplant]

Hey @JavierJF!

I'm also observing high CPU usage on the ProxySQL instance, even when idling. Could this somehow be related to the same issue or it's unrelated?

To clarify:
I still have one instance running with 3.0.1 without restarting it, but it's not processing production traffic.

proxysql 4106365 53.8  2.5 9263868 6753836

Memory usage is high, but not sure if CPU usage can also be related to this issue, could it be?

Is there a way to check what's is doing, besides processlist? I checked also with strace but didn't spot anything useful.

[JavierJF]

Hi @cyberplant,

sorry I missed this one. Half a core seems like too much for a idling instance. CPU usage shouldn't be related to this issue, specially since according to the ps you shared the instance has plenty of memory available, please correct me otherwise. Said this, I'm not sure if an overhead can be expected in SQLite3 when the number of prepared statements grows very high without freeing.

To be certain that the issue is gone the simplest thing would be to test this patch in your instance, if CPU usage never increases, my guess would be an overhead in SQLite3 motivated from the (very) high of non-closed statements. If on the contrary you still experience the CPU overhead, I would suggest opening a different issue and sharing the details of your setup. We could follow up there to understand the reason of the overhead.

Thanks, regards,
Javier.

[cyberplant]

Excellent, I'll test this and if I see the issue still happening, I'll open a new ticket with some data for investigation.

For testing your PR, does the CI/CD have some package built for me, or I have to build the package by myself from this branch?

[JavierJF]

Hi @cyberplant,

I can provide you with builds for this PR if you require them, you can write me the details of your OS for the build at [email protected]. I will share the build for testing privately there.

Regards, Javier.

[cyberplant]

Just FTR: Had the test build running for some days, and the leak fix looks good. No increase in memory as it was happening before.

Today I put the server in production to check how it goes with production load and still looks good. 👍

[JavierJF]

Hi @cyberplant,

thanks for testing the build and confirming that CPU increase was likely a side effect.

Regards, Javier.

[cyberplant]

Hi @cyberplant,

thanks for testing the build and confirming that CPU increase was likely a side effect.

Memory usage has been stable now with the test build, but the CPU usage is still increasing, so I would say it's not related to this issue. For example, yesterday the query select * from stats.stats_memory_metrics; took 0.01 seconds (sometimes 0.00) and now it takes 0.03 consistently. I will investigate more and open a new issue for this.

Thanks!