Skip to content

Fix: memory leak in XML::Node#content=#16419

Merged
straight-shoota merged 1 commit intocrystal-lang:masterfrom
toddsundsted:save-next-pointer-before-unlinking
Nov 27, 2025
Merged

Fix: memory leak in XML::Node#content=#16419
straight-shoota merged 1 commit intocrystal-lang:masterfrom
toddsundsted:save-next-pointer-before-unlinking

Conversation

@toddsundsted
Copy link
Contributor

@toddsundsted toddsundsted commented Nov 25, 2025

This PR saves the next pointer before unlinking, as xmlUnlinkNode clears it. In practice, this means the first child is added to the list of unlinked nodes, but all siblings are left off and become leaks.

Sorry for sending these one-by-one. I'm encountering them as I'm stress testing my LibXML extensions. Also, I created this PR against a branch this time, instead of master, hopefully ending the cycle of my nuking the previous PRs.

require "spec"
require "xml"

describe "XML memory management" do
  it "properly unlinks children" do
    xml = %Q|<root>#{"<child/>" * 10}</root>|

    GC.collect
    baseline_rss = get_rss_kb

    1_000_000.times do
      doc = XML.parse(xml)
      root = doc.root.not_nil!
      root.content = "text"
    end

    GC.collect
    final_rss = get_rss_kb
    growth = final_rss - baseline_rss

    p growth: growth

    (growth < 50_000).should be_true
  end
end

private def get_rss_kb : Int64
  {% if flag?(:darwin) %}
    `ps -o rss= -p #{Process.pid}`.strip.to_i64
  {% else %}
    File.read("/proc/self/status").lines.each do |line|
      return line.split[1].to_i64 if line.starts_with?("VmRSS:")
    end
    0_i64
  {% end %}
end

@ysbaddaden ysbaddaden changed the title Save next Pointer Before Unlinking Fix: memory leak in XML::Node#content= Nov 25, 2025
@ysbaddaden ysbaddaden added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:serialization labels Nov 25, 2025
@straight-shoota
Copy link
Member

It's great to have them as individual patches 👍
That means each change is clearly isolated and easy to comprehend.

@straight-shoota straight-shoota added this to the 1.19.0 milestone Nov 25, 2025
@straight-shoota straight-shoota merged commit 093c765 into crystal-lang:master Nov 27, 2025
45 of 46 checks passed
toddsundsted added a commit to toddsundsted/ktistec that referenced this pull request Feb 24, 2026
toddsundsted added a commit to toddsundsted/ktistec that referenced this pull request Mar 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:serialization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants