[PATCH] Fix libxml leaks in contrib/xml2 XPath functions

First seen: 2026-05-31 22:01:24+00:00 · Messages: 5 · Participants: 2

Latest Update

2026-06-04 · claude-opus-4-6

Incremental Update: Thread Progresses Toward Merged Resolution

Summary

Michael Paquier (committer) acknowledged the original patches, identified two additional leak sites, and produced a v2 patch series. Andrey Chernyshov responded with a further incremental fix (v2-0004) addressing a cleanup gap in pgxml_xpath() itself. The thread converged on a merged v2-0001..v2-0004 as the final solution, with an upstream libxml2 API limitation noted but deemed out-of-scope for PostgreSQL.

Key Technical Developments

Two additional error-path leaks identified by Michael (v2 series):

  1. pgxmlNodeSetToText() — missing xmlFree() for the result variable returned by xmlXPathCastNodeToString()
  2. xmlXPathCtxtCompile() in pgxml_xpath() — if pg_xml_error_occurred() fires after a successful allocation, the compiled expression leaks

New cleanup gap in pgxml_xpath() identified by Andrey (v2-0004):

The problem: pgxml_xpath() builds an xpath_workspace struct before returning it to callers. Callers wrap calls to pgxml_xpath() in PG_TRY/PG_CATCH, but if an ERROR is thrown inside pgxml_xpath() before it returns, the caller's workspace pointer was never assigned — so the caller's PG_CATCH cannot invoke cleanup_workspace(). The fix adds local cleanup within pgxml_xpath() itself, tracking comppath for the error path and null-checking xmlXPathNewContext() before dereferencing.

The repro script demonstrates ~83 MB/iteration growth on error paths without the fix (XPath syntax errors in a loop), plateauing completely with the fix applied.

Upstream libxml2 API limitation discussed:

Michael identified that xmlXPathCompiledEval() returns NULL both for a valid empty result AND for internal OOM (the chain goes through xmlXPathCompParserContext() which may fail on xmlMalloc()). PostgreSQL cannot distinguish these cases. Both participants agreed this is an upstream issue requiring a new API with explicit error codes, and that PostgreSQL's xml2 code is doing the best it can given the current interface.

Double-cleanup safety:

Michael noted that with v2-0004, cleanup_workspace() could theoretically be called twice (once in pgxml_xpath's local PG_CATCH, once in the caller's PG_CATCH). He confirmed this is safe — the function handles being called on already-cleaned-up state.

Resolution Status

Michael indicated he will merge v2-0001 through v2-0004 into a final commit after a final review pass of all callers. The back-patching question remains: the original commit 732061150b0 was HEAD-only because error-path leaks were considered not worth back-branch churn, but the success-path leaks from the original report are more operationally significant.

History (1 prior analysis)
2026-06-01 · claude-opus-4-6

Technical Analysis: Fix libxml Leaks in contrib/xml2 XPath Functions

Core Problem

The contrib/xml2 module contains two memory leak bugs on successful execution paths in its XPath evaluation functions. These are not error-handling gaps but fundamental ownership-model violations when interfacing with libxml2's memory management.

The libxml2 Memory Model

libxml2 uses its own allocator (defaulting to malloc/free but overridable). Objects allocated by libxml2 functions must be freed using libxml2's deallocation functions (xmlFree(), xmlXPathFreeObject(), etc.) — they are not managed by PostgreSQL's memory context system. This means:

  1. They are invisible to PostgreSQL's palloc-based memory management
  2. They survive memory context resets
  3. They accumulate across repeated function calls within a single backend session

Leak #1: xpath_list() — String Ownership Confusion

In xpath_list(), the code calls xmlXPathCastNodeToString() which returns a newly-allocated xmlChar *. This pointer is passed directly to xmlBufferWriteCHAR(), which copies the string into the buffer rather than taking ownership. The original xmlChar * is never freed.

This is a classic ownership-model misunderstanding: the caller assumes the callee consumes the allocation, but the callee only reads it. Every XPath node result in xpath_list() leaks one libxml string allocation.

Leak #2: xpath_table() — Two Distinct Leak Sites

The xpath_table() function has two problems:

  1. XPath result objects: xmlXPathCompiledEval() returns an xmlXPathObjectPtr that must be freed with xmlXPathFreeObject(). The function was not doing this after each column evaluation, leaking the entire XPath result structure per column per row.

  2. String values in the values array: The function stores xmlChar * strings (libxml-allocated) in the values array that feeds BuildTupleFromCStrings(). Since BuildTupleFromCStrings() copies/converts these strings into the result tuple (using palloc'd memory), the original libxml strings become garbage that was never freed.

Architectural Significance

Why This Matters for Long-Running Backends

PostgreSQL backends are long-lived processes. Memory leaked outside the palloc system accumulates indefinitely — it cannot be reclaimed by memory context resets, transaction boundaries, or even explicit RESET commands. The only remedy is backend termination.

The reproduction scripts demonstrate steady ~1.3-2 MB/iteration growth in backend RSS for xpath_table and similar growth for xpath_list. In production scenarios with connection pooling (where backends serve thousands of queries), this creates unbounded memory growth leading to OOM conditions.

Relationship to Recent Error-Handling Work

The author correctly identifies that commit 732061150b0 (from BUG #18943) addressed error-path cleanup — ensuring libxml resources are freed when exceptions occur. These patches address the complementary problem: resources that leak on the success path. Both are necessary for correct resource management.

Proposed Solution

Patch 0001: xpath_list Fix

The fix is straightforward: capture the return value of xmlXPathCastNodeToString() in a local variable, pass it to xmlBufferWriteCHAR(), then call xmlFree() on it. This is a minimal, low-risk change.

Patch 0002: xpath_table Fix

This patch is more involved:

  1. Free XPath result objects immediately after extraction: After each xmlXPathCompiledEval() call, once the needed value is extracted from the result, call xmlXPathFreeObject().

  2. Free per-column strings after tuple construction: After BuildTupleFromCStrings() has consumed the values array, iterate through and xmlFree() any libxml-allocated strings.

  3. Track allocations across PG_TRY blocks: The existing error-handling block is extended to also free any in-flight libxml allocations if an error occurs mid-evaluation. This prevents the leaks from simply moving to the error path.

Design Considerations

Back-patching Suitability

The author argues these are candidates for back-patching to all supported branches. This is well-justified because:

  • The leaks are long-standing (present since the functions were written)
  • The fixes are minimal and low-risk
  • Memory leaks in production backends are operationally significant
  • The changes don't alter any external behavior or API

Why These Weren't Caught Earlier

contrib/xml2 is a legacy module (predating the core xml type's XPath support). It receives less attention than core code, and memory leaks from foreign allocators are invisible to PostgreSQL's memory accounting infrastructure and tools like MemoryContextStats().