Hangs forever when trying to replace a child node with the parent node

[sebastianiorga]

Describe the bug

If you(stupidly) try to replace a child node of a parent node with the parent node itself, nokogiri just hangs forever. I traced the issue into xml_node.c I believe it's the reparent_node_with function that hangs, but I'm not a C dev so didn't dig deeper.

ctrl-c also doesn't work, you need to kill the ruby process to close it.

To Reproduce

Run the script below.

#! /usr/bin/env ruby

require 'nokogiri'

parent = Nokogiri::HTML.fragment <<~HTML
  <div class="parent">
    <div class="child">
    </div>
  </div>
HTML

child = parent.css('.child').first
puts 'next line we run hangs'
parent.replace(child.replace(parent))
puts 'never gets here since above line hangs'

Expected behavior

Crash with some sort of error message like: 'Don't do this you dummy.' Presumably this affects more complicated types of accidentally recursive replacements as well, though I haven't checked those.

Environment

Please paste the output from nokogiri -v here, escaped by triple-backtick.

# Nokogiri (1.8.1)
    ---
    warnings: []
    nokogiri: 1.8.1
    ruby:
      version: 2.4.5
      platform: x86_64-linux
      description: ruby 2.4.5p335 (2018-10-18 revision 65137) [x86_64-linux]
      engine: ruby
    libxml:
      binding: extension
      source: packaged
      libxml2_path: "/home/seb/.rvm/gems/ruby-2.4.5/gems/nokogiri-1.8.1/ports/x86_64-pc-linux-gnu/libxml2/2.9.5"
      libxslt_path: "/home/seb/.rvm/gems/ruby-2.4.5/gems/nokogiri-1.8.1/ports/x86_64-pc-linux-gnu/libxslt/1.1.30"
      libxml2_patches: []
      libxslt_patches: []
      compiled: 2.9.5
      loaded: 2.9.5

[flavorjones]

@sebastianiorga How fascinating! Thanks for reporting, I'll definitely take a look. If possible I'd like to raise an exception if an infinite loop like this is going to occur.

[kapcod]

The same happens with parent.parent = child.
Any progress on this?

[flavorjones]

Any progress on this?

No.

[flavorjones]

We'll fix this in v1.11.0, likely with a check that a parent's ancestors don't include the current node (which will be a performance penalty).

[flavorjones]

See #2311