Skip to content

Fix leaks in is_well_formed_xpath and c14n inclusive prefixes - #222

Merged
dginev merged 1 commit into
mainfrom
fix/memory-leaks
Oct 7, 2026
Merged

dginev merged 1 commit into
mainfrom
fix/memory-leaks

Conversation

@dginev

@dginev dginev commented Oct 7, 2026

Copy link
Copy Markdown
Member

Stacked on #221. Merge #220 → #221 → this. Its CHANGELOG lines go into the 0.3.22 section.

Leak audit

I ran every test binary under valgrind --leak-check=full, and statically checked every libxml2 call whose result the caller must free. Two library leaks turned up:

Leak Size Fix
xpath::is_well_formed_xpath 8 libxml2 allocations per call The compiled expression was released with a plain free, leaking its steps. Now uses xmlXPathFreeCompExpr
Document::canonicalize with inclusive_ns_prefixes 1 Rust allocation per prefix per call The CString::into_raw strings were never reclaimed. They now live in a named binding for the call

The other caller-owned results were all freed correctly: serialization buffers, XPath objects and contexts, copied documents, parser contexts, reader strings.

Test without valgrind

tests/leak_tests.rs routes libxml2 through counting allocators (xmlMemSetup) and Rust through a counting global allocator. It asserts that repeated calls add no live allocations, and a guard checks that counting is actually active.

  • Red before each fix: 800 libxml2 allocations over 100 XPath calls; 100 Rust allocations over 50 canonicalizations.
  • Green after, on libxml2 2.9.14 and 2.14.1.

What remains in valgrind is not a library leak

Every remaining leak is a detached node that a test neither re-attaches nor marks set_rust_owned. unlink_node documents that as leaking by contract. These are subtrees the tests unlink, plus nodes from Node::new / create_processing_instruction that are never attached. Cleaning up those tests would let the valgrind job also fail on leaks; that's a follow-up.

The full suite (21 test binaries) is valgrind-clean on libxml2 2.9.14 and 2.14.1.

🤖 Generated with Claude Code

@dginev
dginev force-pushed the fix/init-parser-entry-points branch from 62677ff to 027db0a Compare October 7, 2026 12:14
…ing leak test

Leak audit (every test binary under valgrind --leak-check=full, plus a scan
of caller-owned libxml2 results):

- `is_well_formed_xpath` released the compiled expression with a plain free,
  leaking its steps: 8 libxml2 allocations per call. Now
  `xmlXPathFreeCompExpr`.
- `Document::canonicalize` passed `inclusive_ns_prefixes` as
  `CString::into_raw` strings and never reclaimed them: one Rust allocation
  per prefix per call. The CStrings now live in a named binding for the call.

`tests/leak_tests.rs` needs no valgrind: it routes libxml2 through counting
allocators (`xmlMemSetup`) and Rust through a counting global allocator, and
asserts repeated calls add no live allocations (with a guard that counting is
active). Red before each fix (800 libxml2 allocations over 100 xpath calls;
100 Rust allocations over 50 canonicalizations), green after, on libxml2
2.9.14 and 2.14.1.

Remaining valgrind leaks are all detached nodes the tests neither re-attach
nor mark `set_rust_owned`, which `unlink_node` documents as leaking by
contract; none are library leaks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dginev
dginev changed the base branch from fix/init-parser-entry-points to main October 7, 2026 12:21
@dginev
dginev merged commit c8a993a into main Oct 7, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant