Skip to content

fix: Node::get_namespaces leaked the xmlGetNsList array on every call (0.3.21) - #214

Merged
dginev merged 1 commit into
mainfrom
fix-getnslist-leak
Aug 2, 2026
Merged

fix: Node::get_namespaces leaked the xmlGetNsList array on every call (0.3.21)#214
dginev merged 1 commit into
mainfrom
fix-getnslist-leak

Conversation

@dginev

@dginev dginev commented Aug 2, 2026

Copy link
Copy Markdown
Member

xmlGetNsList mallocs an array of xmlNsPtr the caller must free (the namespaces belong to the doc). The historical TODO tried xmlFreeNs(list) — the first namespace struct, hence the remembered segfault — and was commented out, leaking the array on every call since the crate's beginning. Valgrind on a downstream workload: 2,429 definitely-lost blocks over 30 documents; a leading term of ~150 KB/page RSS growth across a 115,000-page render. Freed via c_helpers::bindgenFree (MSVC-safe). Windows lane verified before merge.

🤖 Generated with Claude Code

The list is a caller-freed malloc'd array of xmlNsPtr; the namespaces it
points at belong to the document. The historical TODO freed it with
xmlFreeNs (the first namespace struct), segfaulted, and was commented out
— leaking the array since the crate's beginning. Measured downstream:
2,429 definitely-lost blocks over 30 documents (valgrind), a leading term
of ~150 KB/page RSS growth across a 115k-page render. Freed via
c_helpers::bindgenFree (MSVC-safe; the xmlFree global does not link
there). 0.3.21.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dginev
dginev merged commit 00a237b into main Aug 2, 2026
18 checks passed
@dginev
dginev deleted the fix-getnslist-leak branch August 2, 2026 13:16
dginev added a commit to dginev/latexml-oxide that referenced this pull request Aug 2, 2026
…s xmlGetNsList array (libxml 0.3.21)

The ~150 KB/page render retention that stopped the joint 131 MB run at
page 56,088 was, dominantly, a leak as old as the rust-libxml crate:
xmlGetNsList returns a caller-freed malloc'd array, and get_namespaces'
historical free attempt (xmlFreeNs on the first namespace struct)
segfaulted and was commented out — leaking the array on EVERY call, one
per materialized node in post's clone/materialization chains. Bisection
trail: memdiag probe split RSS growth into live-C (+78 KB/page) vs flat
C-free/Rust; stage bisection put it in the pass-B skeleton; a parse+drop
probe was clean; a CrossRef probe reproduced (+45 KB/page); valgrind
named xmlGetNsList (2,429 lost blocks / 30 pages). Fixed upstream
(KWARC/rust-libxml#214, Windows lane green pre-merge, 0.3.21 published).

Re-measured render slope: 144 KB/page -> ~4 KB/page (RSS flat at
~2.4 GB through 18k pages). Floor bumped, audit pin refreshed, memdiag
probe (LATEXML_POST_MEMDIAG=1) retained as the standing triage tool.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dginev added a commit to dginev/latexml-oxide that referenced this pull request Aug 2, 2026
…se run (#480)

* fix(status): the combined verdict is the LAST word of every multi-phase run

The framework at ~/git/cortex derives a task's final severity from the last
Status:conversion:N in the log (defaulting to Fatal when absent), and a
human reads the last lines of a run — both audiences were being failed by
the multi-phase executables:

- latexml_oxide ended a post run on 'Info:writer:wrote …' with a silent
  exit(1): the core's 'Conversion failed:' verdict sat thousands of
  per-page post lines up-scroll, so a memory-Fatal'd TRUNCATED site
  masqueraded as success (131 MB witness UAT 2026-08-01: cheap partial cut
  at Ch6, 1,572 plausible pages — the 'suspicious broken label warnings'
  were refs into the amputated remainder). The run now ENDS with the
  combined core+post verdict line.
- latexml_oxide's --log file and archive output carried no terminal
  Status:conversion:N (the zip status member held the human-readable
  core-only message). Both now end with the canonical combined line,
  matching cortex_worker's max(core, post) fold exactly.
- latexmlmath_oxide had no end-of-run fatal guard: a Fatal recovered into
  a printable result exited 0. It now reports and exits 1.
- cortex_worker audited CLEAN (max fold, status-terminated cortex.log,
  Status:conversion:3 fallback zips) — unchanged.

Also fixed, found while root-causing the witness truncation: spill-time
fragment indexing read xml:id in the NAMESPACED form only, which misses
builder-CONSTRUCTED nodes (the plain-attribute form) — 865 of 23,654
spilled labels registered on the witness, silently gutting the fragment
index for label:/id:-scoped rewrites, SegmentMeta ancestor lists, and
spilled-id dedup. Reads now accept both forms (node_xml_id_any_form; the
xmlid ratchet baseline records the reviewed dual-form site).

Guards: 119_final_status_report (final verdict, --log and archive status
contracts, clean-run verdict) + fragment_index::xml_id_readable_in_both_
attribute_forms. Suite 1858/1858, lint gates green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(status): every final-status declaration folds max(REPORT, core, post)

The live REPORT counter is usually the combined value (shared across
phases), but a phase can carry a status FORCED outside it (a
catch_unwind-trapped panic maps to a fatal code without a REPORT
increment). latexml_oxide now threads a phase_status_max through the run
and folds it at every declaration point — the end-of-run verdict, the exit
guard, the --log status line, and the archive status member — exactly
cortex_worker's belt-and-suspenders fold. latexmlmath_oxide documents why
its single-phase REPORT read is already the complete fold.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* refactor(status/stream-split): DRY the verdict surface, expression-oriented front selection

- latexml_core::common::error gains the two canonical formatters —
  conversion_status_line(code) and conversion_verdict(code) — and every
  producer (converter's core-end Note, the CLI's final verdict / --log /
  archive status member, cortex_worker's status line, latexmlmath's guard)
  goes through them, so the format can never drift between executables.
- run_post_processing_inner selects its front-end as an EXPRESSION: both
  arms yield a PostFront (renamed from StreamedFront — it is both arms'
  product), destructured once; drops the four uninitialized lets, the
  out-param assignments and the #[allow(unused_mut)].
- The per-page placeholder probes (needs_index/needs_bib) are one helper
  shared by the streaming sweep and the DOM spill loop.
- stream_split: enclosing_decls() dedupes the level-stack declaration
  gather (page spill + DOM-mode extraction); minor borrow-dance cleanups.

No behavior change: suite 1858/1858, all gates green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(memory): the joint .tex→.html call fits — free what pass B churns, fatal what it can't

The single-invocation 131 MB witness stopped at 56,088 of 115,519 pages:
render RSS grew ~152 KB/page from a ~15 GB core-residual baseline into the
memory fuse. Three coordinated changes:

- drain_pending_xmath_unlinks now FREES the detached XMath subtrees
  (Node::free_subtree) instead of leaking them via the DocOwnedNode
  wrapper — on a math-dense book that leaked entire formula source trees
  per page. PostDocument::drop relinks id-cache handles (Node::set_linked,
  public in libxml 0.3.20) instead of leaking each wrapper allocation.
  The DocOwnedNode module is retired — no users remain.
- The render loop hands freed page memory back to the OS
  (malloc_trim/mi_collect every 512 pages) — the same measured glibc
  free-page retention the core streaming already trims, now applied where
  every page cycles a full DOM + XSLT result through the libc heap.
- All three cooperative memory stops in post (scan sweep, run_phase, the
  render loop) escalate from Error to FATAL severity (user decision
  2026-08-02): a stop that truncates the deliverable must fail the
  combined verdict and the framework-read status, exactly like the core's
  cheap-partial handoff.

libxml floor 0.3.19 → 0.3.20 (published; KWARC/rust-libxml#213, Windows
lane verified green pre-merge); audit pin refreshed. Suite 1857/1857
(doc_owned_node's test retired with the module), all gates green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(memory): the render loop is flat — Node::get_namespaces leaked its xmlGetNsList array (libxml 0.3.21)

The ~150 KB/page render retention that stopped the joint 131 MB run at
page 56,088 was, dominantly, a leak as old as the rust-libxml crate:
xmlGetNsList returns a caller-freed malloc'd array, and get_namespaces'
historical free attempt (xmlFreeNs on the first namespace struct)
segfaulted and was commented out — leaking the array on EVERY call, one
per materialized node in post's clone/materialization chains. Bisection
trail: memdiag probe split RSS growth into live-C (+78 KB/page) vs flat
C-free/Rust; stage bisection put it in the pass-B skeleton; a parse+drop
probe was clean; a CrossRef probe reproduced (+45 KB/page); valgrind
named xmlGetNsList (2,429 lost blocks / 30 pages). Fixed upstream
(KWARC/rust-libxml#214, Windows lane green pre-merge, 0.3.21 published).

Re-measured render slope: 144 KB/page -> ~4 KB/page (RSS flat at
~2.4 GB through 18k pages). Floor bumped, audit pin refreshed, memdiag
probe (LATEXML_POST_MEMDIAG=1) retained as the standing triage tool.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(post): retire the parse_drop_probe triage example

Linux-only (glibc mallinfo2) — it broke the Windows/macOS lanes, and its
mission is complete: the retention it was built to bisect turned out to
be Node::get_namespaces leaking its xmlGetNsList array, fixed upstream
in libxml 0.3.21. The reusable probe is the LATEXML_POST_MEMDIAG hook in
the render loop, which stays.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore(audit): bless psm 0.1.32 / stacker 0.1.25 (diffed, benign)

The gitignored Cargo.lock means CI floats ahead of the local lock; the
lint lane resolved one-patch-newer psm/stacker than the audited pins.
Diffed both bumps: psm is metadata-only (shims byte-identical), stacker's
windows.c is unchanged. Recorded as accepted-version tuples.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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