fix: Node#dup adds the new node to the document's node cache

to make sure it's marked properly. Also, we make sure the new node is decorated if necessary with its document's decorators. Fixes #3359 Co-authored-by: Jean Boussier <[email protected]> (cherry picked from commit c72503a)
sparklemotion · Dec 10, 2024 · 7bf2fc1 · 7bf2fc1
1 parent 765754c
commit 7bf2fc1
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 2 deletions.
diff --git a/ext/nokogiri/xml_node.c b/ext/nokogiri/xml_node.c
@@ -969,6 +969,7 @@ rb_xml_node_initialize_copy_with_args(VALUE rb_self, VALUE rb_other, VALUE rb_le
   xmlNodePtr c_self, c_other;
   int c_level;
   xmlDocPtr c_new_parent_doc;
+  VALUE rb_node_cache;
 
   Noko_Node_Get_Struct(rb_other, xmlNode, c_other);
   c_level = (int)NUM2INT(rb_level);
@@ -980,6 +981,10 @@ rb_xml_node_initialize_copy_with_args(VALUE rb_self, VALUE rb_other, VALUE rb_le
   _xml_node_data_ptr_set(rb_self, c_self);
   noko_xml_document_pin_node(c_self);
 
+  rb_node_cache = DOC_NODE_CACHE(c_new_parent_doc);
+  rb_ary_push(rb_node_cache, rb_self);
+  rb_funcall(rb_new_parent_doc, id_decorate, 1, rb_self);
+
   return rb_self;
 }
 

diff --git a/test/xml/test_document_fragment.rb b/test/xml/test_document_fragment.rb
@@ -301,6 +301,20 @@ def test_dup_creates_tree_with_identical_structure
           assert_equal(original.to_html, duplicate.to_html)
         end
 
+        def test_dup_creates_tree_with_identical_structure_stress
+          # https://github.com/sparklemotion/nokogiri/issues/3359
+          skip_unless_libxml2("this is testing CRuby GC behavior")
+
+          original = Nokogiri::XML::DocumentFragment.parse("<div><p>hello</p></div>")
+          duplicate = original.dup
+
+          stress_memory_while do
+            duplicate.to_html
+          end
+
+          assert_equal(original.to_html, duplicate.to_html)
+        end
+
         def test_dup_creates_mutable_tree
           original = Nokogiri::XML::DocumentFragment.parse("<div><p>hello</p></div>")
           duplicate = original.dup

diff --git a/test/xml/test_node.rb b/test/xml/test_node.rb
@@ -243,15 +243,20 @@ def test_dup_different_parent_document
           doc1 = XML::Document.parse("<root><div><p>hello</p></div></root>")
           doc2 = XML::Document.parse("<div></div>")
 
+          x = Module.new { def awesome!; end }
+          doc2.decorators(XML::Node) << x
+          doc2.decorate!
+
           div = doc1.at_css("div")
           copy = div.dup(1, doc2)
 
           assert_same(doc2, copy.document)
-          assert_equal(1, copy.children.length) # deep copy
+          assert_equal(1, copy.children.length, "expected a deep copy")
+          assert_respond_to(copy, :awesome!, "expected decorators to be copied")
 
           copy = div.dup(0, doc2)
 
-          assert_equal(0, copy.children.length) # shallow copy
+          assert_equal(0, copy.children.length, "expected a shallow copy")
         end
 
         def test_subclass_dup