On Sat, 2007-05-26 at 10:52 +0100, Tom Hughes wrote:
> In message <[EMAIL PROTECTED]>
> [EMAIL PROTECTED] (Jon Burgess) wrote:
>
> > When the attached code is run under valgrind I get the following error.
> >
> > ==13906== Invalid read of size 8
> > ==13906== at 0x6B4E13C: ruby_xml_attr_free (ruby_xml_attr.c:12)
> > ==13906== by 0x3C2524A9CD: rb_gc_call_finalizer_at_exit (gc.c:1928)
> > ==13906== by 0x3C2522FA92: ruby_finalize_1 (eval.c:1548)
> > ==13906== by 0x3C25238BFF: ruby_cleanup (eval.c:1582)
> > ==13906== by 0x3C25238D28: ruby_stop (eval.c:1619)
> > ==13906== by 0x3C252444C3: ruby_run (eval.c:1640)
> > ==13906== by 0x4007E2: main (main.c:48)
> > ==13906== Address 0x6AE4790 is 40 bytes inside a block of size 96 free'd
> > ==13906== at 0x4A055AB: free (vg_replace_malloc.c:233)
> > ==13906== by 0x3C2524A737: ruby_xfree (gc.c:159)
> > ==13906== by 0x3C304507AE: xmlRemoveProp (in
> > /usr/lib64/libxml2.so.2.6.28)
> > ==13906== by 0x6B49C3D: ruby_xml_node_property_set (ruby_xml_node.c:1892)
> > ==13906== by 0x3C25235BAD: rb_call0 (eval.c:5815)
> > ==13906== by 0x3C252360B7: rb_call (eval.c:6062)
> > ==13906== by 0x3C2523DAB3: rb_eval (eval.c:3429)
> > ==13906== by 0x3C2524442A: ruby_exec_internal (eval.c:1608)
> > ==13906== by 0x3C25244474: ruby_exec (eval.c:1628)
> > ==13906== by 0x3C2524449F: ruby_run (eval.c:1638)
> > ==13906== by 0x4007E2: main (main.c:48)
> > ==13906==
> >
> > I think the issue is with ruby_xml_node_property_set() calling
> > xmlRemoveProp() which both removes the attribute from the node and frees
> > it. I think freeing the attribute needs to be conditional on there being
> > no other references.
>
> That is one problem, yes.
>
> It is worse than that though, because if the node which the
> attribute is attached to is freed then the attribute will be
> freed (even if there are other references to it) and there is
> no easy way to find out which attributes need to be kept.
>
> I guess you would have to walk the entire node tree checking
> the _private reference count of each attribute and unlinking
> anything that is shared before freeing the node?
>
> It's horrible anyway...
>
> Tom
>
I'm only just starting to get my head around the interaction between C
and the ruby objects and GC. The patch below seems to be a simple fix
but I'm by no means certain this fixes the more general problems.
I think it works because the GC implicitly performs the reference
checking for the case you mention.
Jon
diff -urw ref/libxml-ruby-0.3.8.4/ext/xml/ruby_xml_node.c libxml-ruby-0.3.8.4/ext/xml/ruby_xml_node.c
--- ref/libxml-ruby-0.3.8.4/ext/xml/ruby_xml_node.c 2006-12-02 10:06:35.000000000 +0000
+++ libxml-ruby-0.3.8.4/ext/xml/ruby_xml_node.c 2007-05-26 16:34:48.000000000 +0100
@@ -1889,7 +1889,7 @@
if( val == Qnil ) {
attr = xmlSetProp(node->node, (xmlChar*)StringValuePtr(key), NULL);
- xmlRemoveProp( attr );
+ xmlUnlinkNode((xmlNodePtr) attr );
return Qnil;
} else {
Check_Type(val, T_STRING);
_______________________________________________
libxml-devel mailing list
libxml-devel@rubyforge.org
http://rubyforge.org/mailman/listinfo/libxml-devel
