Apparent memory leak when duping DocumentFragment

[bryanp]

Ruby 2.1, Nokogiri 1.6.1

Title explains the issue I'm seeing. Here's a simple case to reproduce:

puts `ps -o rss= -p #{$$}`.to_i
doc = Nokogiri::HTML.fragment('<div>foo<span>bar</span></div>')
10000.times do
  doc.dup
  GC.start
end
puts `ps -o rss= -p #{$$}`.to_i

Memory usage goes from 11.9KB to 18.7KB. The contents of the fragment do not seem to matter but the bigger the fragment the bigger the leak (ran it with a large fragment and it peaked at > 1GB).

Having trouble with valgrind on OSX so unable to provide that analysis. It should be clear there's a problem if you run it. Compare to this example, which does not continue to eat up memory (from 11.12KB to 11.16KB):

doc = Nokogiri::HTML::Document.parse('<div>foo<span>bar</span></div>')
puts `ps -o rss= -p #{$$}`.to_i
10000.times do
  doc.dup
  GC.start
end
puts `ps -o rss= -p #{$$}`.to_i

Happy to provide more info if necessary.

[wpaulson]

Also seen on ruby 1.9.3p448 and nokogiri 1.6.0.
First program (fragment() version) prints
18220
18472

Second program (calls parse()) prints
18400
18420

Valgrind results:
Valgrind run on the programs above (valgrind --partial-loads-ok=yes --undef-value-errors=no ruby …) shows the following (Linux version 2.6.18-348.4.1.el5 RedHat)
First program above (calls fragment()):
==27741== Memcheck, a memory error detector
==27741== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==27741== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info
==27741== Command: ruby test.rb
==27741==
87136
91444
==27741==
==27741== HEAP SUMMARY:
==27741== in use at exit: 2,438,775 bytes in 64,678 blocks
==27741== total heap usage: 331,391 allocs, 266,713 frees, 55,918,664 bytes allocated
==27741==
==27741== LEAK SUMMARY:
==27741== definitely lost: 929,962 bytes in 12,530 blocks
==27741== indirectly lost: 946,631 bytes in 32,077 blocks
==27741== possibly lost: 179,997 bytes in 5,134 blocks
==27741== still reachable: 382,185 bytes in 14,937 blocks
==27741== suppressed: 0 bytes in 0 blocks
==27741== Rerun with --leak-check=full to see details of leaked memory
==27741==
==27741== For counts of detected and suppressed errors, rerun with: -v
==27741== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Successful program (calls parse()):
bundle exec valgrind --partial-loads-ok=yes --undef-value-errors=no ruby test.rb
==27587== Memcheck, a memory error detector
==27587== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==27587== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info
==27587== Command: ruby test.rb
==27587==
87504
90124
==27587==
==27587== HEAP SUMMARY:
==27587== in use at exit: 2,390,531 bytes in 64,672 blocks
==27587== total heap usage: 481,268 allocs, 416,596 frees, 71,213,180 bytes allocated
==27587==
==27587== LEAK SUMMARY:
==27587== definitely lost: 919,728 bytes in 12,530 blocks
==27587== indirectly lost: 949,732 bytes in 32,193 blocks
==27587== possibly lost: 155,914 bytes in 5,711 blocks
==27587== still reachable: 365,157 bytes in 14,238 blocks
==27587== suppressed: 0 bytes in 0 blocks
==27587== Rerun with --leak-check=full to see details of leaked memory
==27587==
==27587== For counts of detected and suppressed errors, rerun with: -v
==27587== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

[flavorjones]

Some notes:

• verified this leak with Nokogiri 1.7.0.1 using libxml 2.9.4 with patches
• leak does not occur when re-parsing fragments in a loop
• leak does not occur when duping a full Document
• leak does occur when duping a DocumentFragment

[flavorjones]

OK, this is because DocumentFragment#dup inherits from Node, which is copying the node subtree and rooting it in the doc. These unparented nodes just hang around for the lifetime of the doc.

Suggestion: we could implement DocumentFragment#dup to copy the DocumentFragment's parent Document, and thus treat it more like a full-blown Document (which I think is what people generally expect).

[flavorjones]

I've addressed this in MRI by copying nodes directly to the new Document. However, I'm having trouble with the java implementation. WIP is on branch 1063-flavorjones-dup-document-fragment.

Moved this into the 1.8.3 milestone, sorry for the delay.

[flavorjones]

OK - I've got a second branch up in which I punt on trying to implement this optimization in the JRuby implementaiton. branch is 1063-flavorjones-dup-document-fragment-attempt-2 and I'm going to poke @jvshahid about this decision in the morning.

[flavorjones]

Chatted with @jvshahid and he agrees we should just address this issue in the libxml2/MRI code base and leave JRuby alone, at least for now. I've created #1832 to track this work for JRuby should eventually decide to do it.

Will ship a PR based on the above mentioned branch.