Huge Memory Leak in the Nokogiri::XML::Builder

[julien51]

We're using the Builder in Babylon to build XML stanzas (XMPP).

After running for a few hours each of our application was gaining a lot of weight... It took some hours to track the memory leaks downs (with Dike), and it seems that it comes from the Nokogiri::XML::Builder.

It seems that all the variables in the context of the caller are "wasted", not cleaned after execution.

Could that be possible?

Please feel free to contact me if you have any question about this (julien.genestoux AT gmail.com for Gtalk ;) )

[julien51]

(I am using the master version that I build myself, not the gem from rubygem)

[julien51]

Ok, I have done a little script to reproduce : http://gist.github.com/97446

And the corresponding "log" from Dike : http://gist.github.com/97447

As you'll see, the object that are kept in memory are "complex" object and not regular strings or arrays.

I think I've pretty much reached my limits in terms of technical stuff here, but I think you can see what I mean by "huge" memory leak :)

[julien51]

Ok, I have isolated the memory leak. It comes from the fact that we yield a block. I think this is not good!

So I suggest to get rid of the block in the constructor, but then make calls to the object, like this if you wish

xml = Nokogiri::XML::Builder.new xml.person(:sex => "male", :id => @id) do xml.name(@name) xml.age(@age) xml.city(@city) xml.contacts do @friends.each do |friend| xml.friend(friend) end @family.each do |family| xml.family(family) end end end

I agree to removes some convenience since we have to type xml. for each tag, but I also think it is clearer like this that we're actually "building" something (called xml in this case)

I'll submit a patch later tonight or tomorow since it seems that it's very little work. It's aactually already running here... but I need to make the tests and some cleanup ;)

Let me know what you think!

[julien51]

[julien51]

Alright, done! It is in branch "better-builder" of my version of Nokogiri (I'll do a pull request). Unfortunately I couldn't run the tests (not even before my changes, I get a ton of C errors that I don't quite understand)... but I updated the documentation, and it worked with some tests that I did locally.

Let me know what you think!

[tenderlove]

Okay. First, this isn't a memory leak. A memory leak is when memory is allocated that the Ruby garbage collector will never reclaim. Since Dike walks object space, ruby must know about those objects, so they will get reclaimed at some point. Dike can only tell you about long lived objects.

The problem is that the do .. end block is a closure and closures must hold references to their state at the time. That is why you're seeing references to objects sitting around. This is actually expected behavior for closures.

Nokogiri's builder already supports a non-closure form. I've updated your gist to demonstrate:

http://gist.github.com/97637

[julien51]

But why are those object never cleared then?

[tenderlove]

They will be at some point. You can't force the garbage collector to clean things up. Dike may tell the garbage collector to start, but that doesn't mean it will actually do anything.

[julien51]

Hum, well, I've been running the code in the gist for all night (10hrs) and the script kept eating memory... without releasing it ever. I really think this isn't normal.

Isn't there any solution?

[tenderlove]

Yes, I responded with a solution here:

http://github.com/tenderlove/nokogiri/issues/closed/#issue/18/comment/809

Check out the fork of your gist. It doesn't use the closure. :-)

Also, can you graph the memory that gets eaten? How large do the processes grow over time? Does it continue to grow forever? Does the rate of growth stay the same? How many requests are processed per amount of memory consumed?

[julien51]

but I get an error when I run your gist :/... Since the context is lost with xml.contacts do |contact|

Yes, it continues to grow forever (at least for a night ;)) And yes, the growth rate seams to be pretty linear.

I'll do more stats to giev you more details. (do you use IM?)

[julien51]

And, hum the @context.myvar is really ugly ;)

[julien51]

Ok, my mistake, I kind of didn't understand what you wrote... now it's better. The doc should probably show that there is way to avoid using the closure thingy.

Sorry for my ignorance.

Last thing : it is not very "user-friendly" to add the do |xml| for each block that is being created, is it?

[tenderlove]

sorry! I just pushed another version of the gist:

http://gist.github.com/97637

Ya, you're right, but there isn't a good way to test arity without breaking the API. I'll see what I can do. :-(

It is quite possible there is a leak. I will do more tests using valgrind, but I should have caught any leaks like that in the tests.

I'm on gtalk. My IM is aaron.patterson@gmail.com. Feel free to contact me, but I will be intermittent today as I've got to repair my scooters.

[tenderlove]

Okay, so running with valgrind indicates that there aren't any memory leaks. That means that ruby's garbage collector is freeing up all memory that nokogiri allocates. Valgrind complained about possible memory leaks stemming from the instance_variable_set calls and the define_method calls.