| From: | Andrey Chernyy <andrey(dot)cherny(at)tantorlabs(dot)com> |
|---|---|
| To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: [PATCH] Fix libxml leaks in contrib/xml2 XPath functions |
| Date: | 2026-06-02 00:43:14 |
| Message-ID: | 20260602034314.7eadc4d7@andrnote |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Michael,
I checked v2, and the two additional error-path cases make sense to me.
While looking at pgxml_xpath(), I noticed one more nearby cleanup gap.
pgxml_xpath() builds an xpath_workspace before returning it to callers.
The callers already wrap pgxml_xpath() with PG_TRY/PG_CATCH, but if an
ERROR is thrown before pgxml_xpath() returns, the assignment of the
workspace pointer in the caller has not completed yet. The caller-side
PG_CATCH blocks therefore cannot call cleanup_workspace() for the
partially-built workspace.
Attached is an incremental patch on top of your v2-0001..v2-0003. It
adds local cleanup in pgxml_xpath(), tracks comppath for the error path,
and checks xmlXPathNewContext() before dereferencing the returned
context. I kept it separate for review, but it can of course be folded
if that makes the final series cleaner.
I also used the attached manual repro script, which catches repeated
XPath syntax errors in a single backend and samples VmRSS from /proc.
On my checkout without this pgxml_xpath() cleanup it kept growing after
each round, for example:
postgres=# \i xml2-pgxml-xpath-error-leak-repro.sql
NOTICE: error i=1, failures=20, total_kb=109748, diff_kb=88008
NOTICE: error i=2, failures=20, total_kb=193016, diff_kb=83268
NOTICE: error i=3, failures=20, total_kb=276220, diff_kb=83204
NOTICE: error i=4, failures=20, total_kb=359428, diff_kb=83208
NOTICE: error i=5, failures=20, total_kb=442632, diff_kb=83204
NOTICE: error i=6, failures=20, total_kb=525840, diff_kb=83208
NOTICE: error i=7, failures=20, total_kb=609044, diff_kb=83204
NOTICE: error i=8, failures=20, total_kb=692252, diff_kb=83208
NOTICE: error i=9, failures=20, total_kb=775456, diff_kb=83204
NOTICE: error i=10, failures=20, total_kb=858664, diff_kb=83208
With this patch applied, it plateaued after the initial warmup:
postgres=# \i xml2-pgxml-xpath-error-leak-repro.sql
NOTICE: error i=1, failures=20, total_kb=22672, diff_kb=972
NOTICE: error i=2, failures=20, total_kb=22692, diff_kb=20
NOTICE: error i=3, failures=20, total_kb=22704, diff_kb=8
NOTICE: error i=4, failures=20, total_kb=22700, diff_kb=-8
NOTICE: error i=5, failures=20, total_kb=22704, diff_kb=4
NOTICE: error i=6, failures=20, total_kb=22704, diff_kb=0
NOTICE: error i=7, failures=20, total_kb=22708, diff_kb=4
NOTICE: error i=8, failures=20, total_kb=22704, diff_kb=-8
NOTICE: error i=9, failures=20, total_kb=22708, diff_kb=4
NOTICE: error i=10, failures=20, total_kb=22708, diff_kb=-4
--
Andrey Chernyy
| Attachment | Content-Type | Size |
|---|---|---|
| xml2-pgxml-xpath-error-leak-repro.sql | application/sql | 1.4 KB |
| v2-0004-xml2-Avoid-libxml-leaks-in-pgxml_xpath-error-path.patch | text/x-patch | 3.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | jian he | 2026-06-02 00:41:04 | Re: [PATCH] Rebuild CHECK constraints after generated column SET EXPRESSION |