diff --git a/ext/libxml/ruby_xml_document.c b/ext/libxml/ruby_xml_document.c index d51511b2..5a9083d6 100644 --- a/ext/libxml/ruby_xml_document.c +++ b/ext/libxml/ruby_xml_document.c @@ -68,9 +68,19 @@ void rxml_document_free(void* data) xmlFreeDoc(xdoc); } +/* GC compaction moves the Ruby wrapper but does not update the address that + the registry holds for this document. The document pointer is the registry + key, so ask the registry to rewrite this entry with the new address. */ +static void rxml_document_compact(void* data) +{ + xmlDocPtr xdoc = (xmlDocPtr)data; + if (xdoc) + rxml_registry_update(xdoc); +} + const rb_data_type_t rxml_document_data_type = { .wrap_struct_name = "LibXML::XML::Document", - .function = { .dmark = NULL, .dfree = rxml_document_free }, + .function = { .dmark = NULL, .dfree = rxml_document_free, .dcompact = rxml_document_compact }, .flags = RUBY_TYPED_FREE_IMMEDIATELY, }; diff --git a/ext/libxml/ruby_xml_html_parser_context.c b/ext/libxml/ruby_xml_html_parser_context.c index 28bbed1a..23c9ff02 100644 --- a/ext/libxml/ruby_xml_html_parser_context.c +++ b/ext/libxml/ruby_xml_html_parser_context.c @@ -13,7 +13,6 @@ */ VALUE cXMLHtmlParserContext; -static ID IO_ATTR; /* OS X 10.5 ships with libxml2 version 2.6.16 which does not expose the htmlNewParserCtxt (or htmlInitParserCtxt which it uses) method. htmlNewParserCtxt @@ -129,15 +128,27 @@ static htmlParserCtxtPtr htmlNewParserCtxt(void) } #endif +/* XML::HTMLParser::Context.io stores the read context in the _private field + of the libxml parser context. libxml2 never touches that field. */ static void rxml_html_parser_context_free(void* data) { htmlParserCtxtPtr ctxt = (htmlParserCtxtPtr)data; + if (!ctxt) return; + rxml_io_context_free((rxml_io_context*)ctxt->_private); + ctxt->_private = NULL; htmlFreeParserCtxt(ctxt); } +static void rxml_html_parser_context_mark(void* data) +{ + htmlParserCtxtPtr ctxt = (htmlParserCtxtPtr)data; + if (!ctxt) return; + rxml_io_context_mark((rxml_io_context*)ctxt->_private); +} + const rb_data_type_t rxml_html_parser_context_type = { "LibXML::XML::HTMLParser::Context", - {NULL, rxml_html_parser_context_free, NULL}, + {rxml_html_parser_context_mark, rxml_html_parser_context_free, NULL}, &rxml_parser_context_type, NULL, 0 }; @@ -197,13 +208,22 @@ static VALUE rxml_html_parser_context_io(int argc, VALUE* argv, VALUE klass) if (NIL_P(io)) rb_raise(rb_eTypeError, "Must pass in an IO object"); - input = xmlParserInputBufferCreateIO((xmlInputReadCallback) rxml_read_callback, NULL, - (void*)io, XML_CHAR_ENCODING_NONE); - ctxt = htmlNewParserCtxt(); if (!ctxt) + rxml_raise(xmlGetLastError()); + + /* libxml2 keeps the context pointer and calls the read callback much later. + So give libxml2 a stable address instead of the io object itself. The + mark function marks the io object, which keeps it alive and stops GC + compaction from moving it. */ + ctxt->_private = rxml_io_context_new(io); + + input = xmlParserInputBufferCreateIO((xmlInputReadCallback) rxml_read_callback, NULL, + ctxt->_private, XML_CHAR_ENCODING_NONE); + + if (!input) { - xmlFreeParserInputBuffer(input); + rxml_html_parser_context_free(ctxt); rxml_raise(xmlGetLastError()); } @@ -218,14 +238,13 @@ static VALUE rxml_html_parser_context_io(int argc, VALUE* argv, VALUE klass) if (!stream) { xmlFreeParserInputBuffer(input); - xmlFreeParserCtxt(ctxt); + rxml_html_parser_context_free(ctxt); rxml_raise(xmlGetLastError()); } inputPush(ctxt, stream); result = rxml_html_parser_context_wrap(ctxt); - /* Attach io object to parser so it won't get freed.*/ - rb_ivar_set(result, IO_ATTR, io); + RB_GC_GUARD(io); return result; } @@ -351,7 +370,6 @@ static VALUE rxml_html_parser_context_alloc(VALUE klass) void rxml_init_html_parser_context(void) { - IO_ATTR = ID2SYM(rb_intern("@io")); cXMLHtmlParserContext = rb_define_class_under(cXMLHtmlParser, "Context", cXMLParserContext); rb_define_alloc_func(cXMLHtmlParserContext, rxml_html_parser_context_alloc); diff --git a/ext/libxml/ruby_xml_io.c b/ext/libxml/ruby_xml_io.c index 727453e7..272294e8 100644 --- a/ext/libxml/ruby_xml_io.c +++ b/ext/libxml/ruby_xml_io.c @@ -6,12 +6,36 @@ static ID READ_METHOD; static ID WRITE_METHOD; +rxml_io_context* rxml_io_context_new(VALUE io) +{ + rxml_io_context* context = ALLOC(rxml_io_context); + context->io = io; + return context; +} + +void rxml_io_context_init(rxml_io_context* context, VALUE io) +{ + context->io = io; +} + +void rxml_io_context_free(rxml_io_context* context) +{ + if (context) + xfree(context); +} + +void rxml_io_context_mark(rxml_io_context* context) +{ + if (context && !NIL_P(context->io)) + rb_gc_mark(context->io); +} + /* This method is called by libxml when it wants to read more data from a stream. We go with the duck typing solution to support StringIO objects. */ int rxml_read_callback(void *context, char *buffer, int len) { - VALUE io = (VALUE) context; + VALUE io = ((rxml_io_context*) context)->io; VALUE string = rb_funcall(io, READ_METHOD, 1, INT2NUM(len)); size_t size; diff --git a/ext/libxml/ruby_xml_io.h b/ext/libxml/ruby_xml_io.h index cafd95ce..d64ab62f 100644 --- a/ext/libxml/ruby_xml_io.h +++ b/ext/libxml/ruby_xml_io.h @@ -3,6 +3,27 @@ #ifndef __RXML_IO__ #define __RXML_IO__ +/* Context that libxml2 receives for reads from a Ruby IO object. + + libxml2 keeps the context pointer and calls rxml_read_callback much later. + A raw VALUE is not a valid context, because GC compaction moves the IO + object and the VALUE then holds a dead address. So give libxml2 the + address of this struct instead. + + The owner of the struct MUST call rxml_io_context_mark from its dmark + function. rb_gc_mark pins the IO object, so the VALUE in the struct stays + correct. The owner MUST also call rxml_io_context_free from its dfree + function. */ +typedef struct +{ + VALUE io; +} rxml_io_context; + +rxml_io_context* rxml_io_context_new(VALUE io); +void rxml_io_context_init(rxml_io_context* context, VALUE io); +void rxml_io_context_free(rxml_io_context* context); +void rxml_io_context_mark(rxml_io_context* context); + int rxml_read_callback(void *context, char *buffer, int len); int rxml_write_callback(VALUE io, const char *buffer, int len); void rxml_init_io(void); diff --git a/ext/libxml/ruby_xml_node.c b/ext/libxml/ruby_xml_node.c index e8710ae7..8f53f94d 100644 --- a/ext/libxml/ruby_xml_node.c +++ b/ext/libxml/ruby_xml_node.c @@ -96,9 +96,20 @@ const rb_data_type_t rxml_node_unmanaged_data_type = { .flags = RUBY_TYPED_FREE_IMMEDIATELY, }; +/* GC compaction moves the Ruby wrapper but does not update the address that + the registry holds for this node. The node pointer is the registry key, so + ask the registry to rewrite this entry with the new address. Only managed + nodes are in the registry, so only this type needs the function. */ +static void rxml_node_compact(void* data) +{ + xmlNodePtr xnode = (xmlNodePtr)data; + if (xnode) + rxml_registry_update(xnode); +} + static const rb_data_type_t rxml_node_managed_data_type = { .wrap_struct_name = "LibXML::XML::Node (managed)", - .function = { .dmark = (RUBY_DATA_FUNC)rxml_node_mark, .dfree = rxml_node_free }, + .function = { .dmark = (RUBY_DATA_FUNC)rxml_node_mark, .dfree = rxml_node_free, .dcompact = rxml_node_compact }, .parent = &rxml_node_data_type, .flags = RUBY_TYPED_FREE_IMMEDIATELY, }; diff --git a/ext/libxml/ruby_xml_parser_context.c b/ext/libxml/ruby_xml_parser_context.c index 610a88d8..97ccefff 100644 --- a/ext/libxml/ruby_xml_parser_context.c +++ b/ext/libxml/ruby_xml_parser_context.c @@ -6,7 +6,6 @@ #include VALUE cXMLParserContext; -static ID IO_ATTR; /* * Document-class: LibXML::XML::Parser::Context @@ -15,15 +14,27 @@ static ID IO_ATTR; * a document is parsed. */ +/* XML::Parser::Context.io stores the read context in the _private field of + the libxml parser context. libxml2 never touches that field. */ static void rxml_parser_context_free(void* data) { xmlParserCtxtPtr ctxt = (xmlParserCtxtPtr)data; + if (!ctxt) return; + rxml_io_context_free((rxml_io_context*)ctxt->_private); + ctxt->_private = NULL; xmlFreeParserCtxt(ctxt); } +static void rxml_parser_context_mark(void* data) +{ + xmlParserCtxtPtr ctxt = (xmlParserCtxtPtr)data; + if (!ctxt) return; + rxml_io_context_mark((rxml_io_context*)ctxt->_private); +} + const rb_data_type_t rxml_parser_context_type = { "LibXML::XML::Parser::Context", - {NULL, rxml_parser_context_free, NULL}, + {rxml_parser_context_mark, rxml_parser_context_free, NULL}, NULL, NULL, 0 }; @@ -158,14 +169,23 @@ static VALUE rxml_parser_context_io(int argc, VALUE* argv, VALUE klass) if (NIL_P(io)) rb_raise(rb_eTypeError, "Must pass in an IO object"); - xmlParserInputBufferPtr input = xmlParserInputBufferCreateIO((xmlInputReadCallback) rxml_read_callback, NULL, - (void*)io, XML_CHAR_ENCODING_NONE); - xmlParserCtxtPtr ctxt = xmlNewParserCtxt(); if (!ctxt) + rxml_raise(xmlGetLastError()); + + /* libxml2 keeps the context pointer and calls the read callback much later. + So give libxml2 a stable address instead of the io object itself. The + mark function marks the io object, which keeps it alive and stops GC + compaction from moving it. */ + ctxt->_private = rxml_io_context_new(io); + + xmlParserInputBufferPtr input = xmlParserInputBufferCreateIO((xmlInputReadCallback) rxml_read_callback, NULL, + ctxt->_private, XML_CHAR_ENCODING_NONE); + + if (!input) { - xmlFreeParserInputBuffer(input); + rxml_parser_context_free(ctxt); rxml_raise(xmlGetLastError()); } @@ -180,14 +200,14 @@ static VALUE rxml_parser_context_io(int argc, VALUE* argv, VALUE klass) if (!stream) { xmlFreeParserInputBuffer(input); - xmlFreeParserCtxt(ctxt); + rxml_parser_context_free(ctxt); rxml_raise(xmlGetLastError()); } inputPush(ctxt, stream); + VALUE result = rxml_parser_context_wrap(ctxt); - /* Attach io object to parser so it won't get freed.*/ - rb_ivar_set(result, IO_ATTR, io); + RB_GC_GUARD(io); return result; } @@ -960,8 +980,6 @@ static VALUE rxml_parser_context_well_formed_q(VALUE self) void rxml_init_parser_context(void) { - IO_ATTR = ID2SYM(rb_intern("@io")); - cXMLParserContext = rb_define_class_under(cXMLParser, "Context", rb_cObject); rb_define_alloc_func(cXMLParserContext, rxml_parser_context_alloc); diff --git a/ext/libxml/ruby_xml_reader.c b/ext/libxml/ruby_xml_reader.c index 7475b0e0..d0e80bba 100644 --- a/ext/libxml/ruby_xml_reader.c +++ b/ext/libxml/ruby_xml_reader.c @@ -57,24 +57,63 @@ VALUE cXMLReader; static ID BASE_URI_SYMBOL; static ID ENCODING_SYMBOL; -static ID IO_ATTR; static ID OPTIONS_SYMBOL; +/* An xmlTextReader is opaque, so there is no field in it for Ruby data. The + Ruby object therefore wraps this struct instead of the reader itself. + + XML::Reader.io gives libxml2 the address of io_context. libxml2 keeps that + address and calls the read callback much later. The mark function marks + io_context.io, which keeps the io object alive and stops GC compaction + from moving it. + + XML::Reader.string gives libxml2 the address of the buffer of a String. + xmlReaderForMemory does not copy the buffer, and libxml2 reads from it on + each call to XML::Reader#read. So the reader keeps the String in the + string field. The mark function marks that String with rb_gc_mark, which + both keeps the String alive and stops GC compaction from moving it. */ +typedef struct +{ + xmlTextReaderPtr xreader; + rxml_io_context io_context; + VALUE string; +} rxml_reader_object; + +static rxml_reader_object* rxml_reader_object_alloc(void) +{ + rxml_reader_object* rro = ALLOC(rxml_reader_object); + rro->xreader = NULL; + rro->string = Qnil; + rxml_io_context_init(&rro->io_context, Qnil); + return rro; +} + static void rxml_reader_free(void* data) { - xmlTextReaderPtr xreader = (xmlTextReaderPtr)data; - xmlFreeTextReader(xreader); + rxml_reader_object* rro = (rxml_reader_object*)data; + if (rro->xreader) + xmlFreeTextReader(rro->xreader); + xfree(rro); } static void rxml_reader_mark(void* data) { - xmlTextReaderPtr xreader = (xmlTextReaderPtr)data; - xmlDocPtr xdoc = xmlTextReaderCurrentDoc(xreader); - if (xdoc) + rxml_reader_object* rro = (rxml_reader_object*)data; + + rxml_io_context_mark(&rro->io_context); + + if (!NIL_P(rro->string)) + rb_gc_mark(rro->string); + + if (rro->xreader) { - VALUE doc = rxml_registry_lookup(xdoc); - if (!NIL_P(doc)) - rb_gc_mark(doc); + xmlDocPtr xdoc = xmlTextReaderCurrentDoc(rro->xreader); + if (xdoc) + { + VALUE doc = rxml_registry_lookup(xdoc); + if (!NIL_P(doc)) + rb_gc_mark(doc); + } } } @@ -84,17 +123,24 @@ static const rb_data_type_t rxml_reader_data_type = { .flags = RUBY_TYPED_FREE_IMMEDIATELY, }; -static VALUE rxml_reader_wrap(xmlTextReaderPtr xreader) +static VALUE rxml_reader_wrap(rxml_reader_object* rro) { - return TypedData_Wrap_Struct(cXMLReader, &rxml_reader_data_type, xreader); + return TypedData_Wrap_Struct(cXMLReader, &rxml_reader_data_type, rro); } +/* Wraps a reader that does not read from a Ruby object. */ +static VALUE rxml_reader_wrap_xreader(xmlTextReaderPtr xreader) +{ + rxml_reader_object* rro = rxml_reader_object_alloc(); + rro->xreader = xreader; + return rxml_reader_wrap(rro); +} static xmlTextReaderPtr rxml_text_reader_get(VALUE obj) { - xmlTextReaderPtr xreader; - TypedData_Get_Struct(obj, xmlTextReader, &rxml_reader_data_type, xreader); - return xreader; + rxml_reader_object* rro; + TypedData_Get_Struct(obj, rxml_reader_object, &rxml_reader_data_type, rro); + return rro->xreader; } /* @@ -115,7 +161,7 @@ VALUE rxml_reader_document(VALUE klass, VALUE doc) if (xreader == NULL) rxml_raise(xmlGetLastError()); - return rxml_reader_wrap(xreader); + return rxml_reader_wrap_xreader(xreader); } /* call-seq: @@ -163,7 +209,7 @@ static VALUE rxml_reader_file(int argc, VALUE *argv, VALUE klass) if (xreader == NULL) rb_syserr_fail(ENOENT, StringValueCStr(path)); - return rxml_reader_wrap(xreader); + return rxml_reader_wrap_xreader(xreader); } /* call-seq: @@ -214,17 +260,20 @@ static VALUE rxml_reader_io(int argc, VALUE *argv, VALUE klass) xoptions = NIL_P(parserOptions) ? 0 : NUM2INT(parserOptions); } + /* Wrap the struct before it holds any VALUE. The garbage collector can + then see the io object through the mark function. */ + rxml_reader_object* rro = rxml_reader_object_alloc(); + result = rxml_reader_wrap(rro); + rxml_io_context_init(&rro->io_context, io); + xreader = xmlReaderForIO((xmlInputReadCallback) rxml_read_callback, NULL, - (void *) io, + &rro->io_context, xbaseurl, xencoding, xoptions); if (xreader == NULL) rxml_raise(xmlGetLastError()); - result = rxml_reader_wrap(xreader); - - /* Attach io object to parser so it won't get freed.*/ - rb_ivar_set(result, IO_ATTR, io); + rro->xreader = xreader; return result; } @@ -277,13 +326,29 @@ static VALUE rxml_reader_string(int argc, VALUE *argv, VALUE klass) xoptions = NIL_P(parserOptions) ? 0 : NUM2INT(parserOptions); } - xreader = xmlReaderForMemory(StringValueCStr(string), (int)RSTRING_LEN(string), + /* Reject a string that contains a null character. */ + StringValueCStr(string); + + /* Wrap the struct before it holds any VALUE. The garbage collector can then + see the string through the mark function. */ + rxml_reader_object* rro = rxml_reader_object_alloc(); + VALUE result = rxml_reader_wrap(rro); + + /* Take a frozen copy of the string. The copy shares the buffer of the + original string, so this does not copy the data of a large document. The + copy also protects the reader if the program changes the original + string. */ + rro->string = rb_str_new_frozen(string); + + xreader = xmlReaderForMemory(RSTRING_PTR(rro->string), (int)RSTRING_LEN(rro->string), xbaseurl, xencoding, xoptions); if (xreader == NULL) rxml_raise(xmlGetLastError()); - return rxml_reader_wrap(xreader); + rro->xreader = xreader; + + return result; } /* @@ -1143,7 +1208,6 @@ void rxml_init_reader(void) { BASE_URI_SYMBOL = ID2SYM(rb_intern("base_uri")); ENCODING_SYMBOL = ID2SYM(rb_intern("encoding")); - IO_ATTR = rb_intern("@io"); OPTIONS_SYMBOL = ID2SYM(rb_intern("options")); cXMLReader = rb_define_class_under(mXML, "Reader", rb_cObject); diff --git a/ext/libxml/ruby_xml_registry.c b/ext/libxml/ruby_xml_registry.c index 7f70e45e..8f81c3ed 100644 --- a/ext/libxml/ruby_xml_registry.c +++ b/ext/libxml/ruby_xml_registry.c @@ -29,3 +29,10 @@ VALUE rxml_registry_lookup(void *ptr) return (VALUE)val; return Qnil; } + +void rxml_registry_update(void *ptr) +{ + st_data_t val; + if (st_lookup(rxml_registry, (st_data_t)ptr, &val)) + st_insert(rxml_registry, (st_data_t)ptr, (st_data_t)rb_gc_location((VALUE)val)); +} diff --git a/ext/libxml/ruby_xml_registry.h b/ext/libxml/ruby_xml_registry.h index 9cfeb68e..58a4c06d 100644 --- a/ext/libxml/ruby_xml_registry.h +++ b/ext/libxml/ruby_xml_registry.h @@ -12,11 +12,18 @@ of reading _private). Registered pointers MUST be unregistered before the underlying C struct is - freed, typically in the wrapper's dfree function. */ + freed, typically in the wrapper's dfree function. + + The stored VALUEs are plain machine addresses, so GC compaction invalidates + them when it moves a wrapper. Each wrapper type that registers itself MUST + also install a dcompact function that calls rxml_registry_update with its + own data pointer. The data pointer is the registry key, so the entry for + the object that moved is the entry that dcompact repairs. */ void rxml_init_registry(void); void rxml_registry_register(void *ptr, VALUE obj); void rxml_registry_unregister(void *ptr); VALUE rxml_registry_lookup(void *ptr); /* Qnil on miss */ +void rxml_registry_update(void *ptr); /* rewrite entry after compaction */ #endif diff --git a/test/test_document.rb b/test/test_document.rb index 231a7d0d..c0aa2fbd 100644 --- a/test/test_document.rb +++ b/test/test_document.rb @@ -135,4 +135,20 @@ def test_io end end + # The bindings map an xmlDocPtr to its Ruby wrapper in a registry. GC + # compaction moves the wrapper. The registry must learn the new address. + # The document stays in a heap container so that compaction can move it. + def test_gc_compaction_updates_document_registry + holder = [LibXML::XML::Document.string('')] + + compact_heap + + assert_equal('root', holder[0].root.doc.root.name) + + # A node wrapper marks its document through the registry. A stale entry + # sends a dead address to the garbage collector. + holder[0].root + GC.start(full_mark: true, immediate_sweep: true) + end + end diff --git a/test/test_helper.rb b/test/test_helper.rb index 1a56fa7a..0e834874 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -14,6 +14,24 @@ def windows? !(RbConfig::CONFIG['host_os'] =~ /mswin|mingw/).nil? end +# Moves every movable object on the heap. +# +# Tests that use this method must keep the object under test in a heap +# container, for example an Array. An object in a live local variable is +# pinned by the conservative machine stack scan, so it never moves and the +# test cannot detect a stale address. +def compact_heap + if GC.respond_to?(:verify_compaction_references) + begin + GC.verify_compaction_references(expand_heap: true, toward: :empty) + rescue NotImplementedError => e + skip("GC compaction is not available: #{e.message}") + end + else + skip('GC compaction is not available') + end +end + STDOUT.write "\nlibxml2: #{LibXML::XML::LIBXML_VERSION}\n#{RUBY_DESCRIPTION}\n\n" require 'minitest/autorun' diff --git a/test/test_html_parser.rb b/test/test_html_parser.rb index e2662c5d..4e901675 100644 --- a/test/test_html_parser.rb +++ b/test/test_html_parser.rb @@ -52,6 +52,18 @@ def test_io_gc assert(parser.parse) end + # libxml2 keeps the read context and calls the read callback at parse time. + # GC compaction moves the io object, so the context must not be a raw + # address. The parser stays in a heap container, not in a local variable, + # so that compaction can move it. + def test_io_gc_compaction + holder = [LibXML::XML::HTMLParser.io(StringIO.new(File.read(html_file)))] + + compact_heap + + assert_instance_of(LibXML::XML::Document, holder[0].parse) + end + def test_nil_io error = assert_raises(TypeError) do LibXML::XML::HTMLParser.io(nil) diff --git a/test/test_node.rb b/test/test_node.rb index 1e23f329..8dd296d8 100644 --- a/test/test_node.rb +++ b/test/test_node.rb @@ -242,6 +242,25 @@ def test_document_node_marks_document GC.stress = false end + # A standalone node owns its libxml tree, so the bindings map the xmlNodePtr + # to its Ruby wrapper in a registry. GC compaction moves the wrapper. The + # registry must learn the new address. The wrapper stays in a heap container + # so that compaction can move it. + def test_gc_compaction_updates_node_registry + holder = [LibXML::XML::Node.new('root')] + holder[0] << LibXML::XML::Node.new('child') + + compact_heap + + # A child node wrapper marks the root wrapper through the registry. A + # stale entry sends a dead address to the garbage collector. + child = holder[0].child + GC.start(full_mark: true, immediate_sweep: true) + + assert_equal('child', child.name) + assert_equal('root', holder[0].name) + end + private def create_document_child diff --git a/test/test_parser.rb b/test/test_parser.rb index f94e54fd..9c584e04 100644 --- a/test/test_parser.rb +++ b/test/test_parser.rb @@ -103,6 +103,18 @@ def test_io_gc assert(parser.parse) end + # libxml2 keeps the read context and calls the read callback at parse time. + # GC compaction moves the io object, so the context must not be a raw + # address. The parser stays in a heap container, not in a local variable, + # so that compaction can move it. + def test_io_gc_compaction + holder = [LibXML::XML::Parser.io(StringIO.new(''))] + + compact_heap + + assert_equal('root', holder[0].parse.root.name) + end + def test_nil_io error = assert_raises(TypeError) do LibXML::XML::Parser.io(nil) diff --git a/test/test_reader.rb b/test/test_reader.rb index 4cd93788..e4d39ada 100644 --- a/test/test_reader.rb +++ b/test/test_reader.rb @@ -88,6 +88,18 @@ def test_io_gc assert(reader.read) end + # libxml2 keeps the read context and calls the read callback on each read. + # GC compaction moves the io object, so the context must not be a raw + # address. The reader stays in a heap container, not in a local variable, + # so that compaction can move it. + def test_io_gc_compaction + holder = [LibXML::XML::Reader.io(StringIO.new(File.read(XML_FILE)))] + + compact_heap + + verify_simple(holder[0]) + end + def test_string_io data = File.read(XML_FILE) string_io = StringIO.new(data) @@ -401,8 +413,34 @@ def test_expand_gc_after_advance GC.stress = false end + # xmlReaderForMemory does not copy the string, and libxml2 reads from the + # buffer of the string on each call to #read. So the reader must keep the + # string. A short literal string does not show the defect, because the + # literal stays in place. The test therefore builds a large string at run + # time and then makes the garbage collector reuse the memory. + def test_string_gc_compaction + holder = [LibXML::XML::Reader.string(build_large_xml)] + + compact_heap + 20_000.times { |i| +"churn #{i}" } + + elements = 0 + while holder[0].read + elements += 1 if holder[0].node_type == LibXML::XML::Reader::TYPE_ELEMENT + end + + assert_equal(201, elements) + end + private + def build_large_xml + xml = +'' + 200.times { xml << "#{'x' * 50}" } + xml << '' + xml + end + def expand_and_advance reader = LibXML::XML::Reader.string("") reader.read # root