Ensure destructors for RcDom nodes are invoked interatively, not recursively.

Author: jdm

html5ever/examples/print-rcdom.rs

@@ -21,7 +21,7 @@ use html5ever::tendril::TendrilSink;
 
 // This is not proper HTML serialization, of course.
 
-fn walk(indent: usize, handle: Handle) {
+fn walk(indent: usize, handle: &Handle) {
     let node = handle;
     // FIXME: don't allocate
     print!("{}", repeat(" ").take(indent).collect::<String>());
@@ -58,7 +58,7 @@ fn walk(indent: usize, handle: Handle) {
     }
 
     for child in node.children.borrow().iter() {
-        walk(indent + 4, child.clone());
+        walk(indent + 4, child);
     }
 }
 
@@ -73,11 +73,11 @@ fn main() {
         .from_utf8()
         .read_from(&mut stdin.lock())
         .unwrap();
-    walk(0, dom.document);
+    walk(0, &dom.document);
 
     if !dom.errors.is_empty() {
         println!("\nParse errors:");
-        for err in dom.errors.into_iter() {
+        for err in dom.errors.iter() {
             println!("    {}", err);
         }
     }

html5ever/tests/tree_builder.rs

@@ -228,7 +228,7 @@ fn make_test_desc_with_scripting_flag(
                         .one(data.clone());
                     // fragment case: serialize children of the html element
                     // rather than children of the document
-                    let doc = dom.document;
+                    let doc = &dom.document;
                     let root = &doc.children.borrow()[0];
                     for child in root.children.borrow().iter() {
                         serialize(&mut result, 1, child.clone());

markup5ever/Cargo.toml

@@ -16,6 +16,7 @@ path = "lib.rs"
 string_cache = "0.7"
 phf = "0.7"
 tendril = "0.4"
+log = "0.4"
 
 [build-dependencies]
 string_cache_codegen = "0.4"

markup5ever/lib.rs

@@ -7,6 +7,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+#[macro_use]
+extern crate log;
 extern crate phf;
 extern crate string_cache;
 pub extern crate tendril;

markup5ever/rcdom.rs

@@ -109,6 +109,8 @@ pub struct Node {
     pub children: RefCell<Vec<Handle>>,
     /// Represents this node's data.
     pub data: NodeData,
+    /// Flag to control whether to free any children on destruction.
+    leak_children_on_drop: Cell<bool>,
 }
 
 impl Node {
@@ -118,8 +120,38 @@ impl Node {
             data: data,
             parent: Cell::new(None),
             children: RefCell::new(Vec::new()),
+            leak_children_on_drop: Cell::new(true),
         })
     }
+
+    /// Drop any child nodes remaining in this node at destruction.
+    ///
+    /// RcDom's destructor automatically drops any nodes and children that are
+    /// present in the document. This setting only affects nodes that are dropped
+    /// by manipulating the tree before RcDom's destructor runs (such as manually
+    /// removing children from a node after parsing is complete).
+    ///
+    /// Unsafety: due to the representation of children, this can trigger
+    /// stack overflow if dropping a node with a very deep tree of children.
+    /// This is not a recommended configuration to use when interacting with
+    /// arbitrary HTML content.
+    pub unsafe fn free_child_nodes_on_drop(&self) {
+        self.leak_children_on_drop.set(false);
+    }
+}
+
+impl Drop for Node {
+    fn drop(&mut self) {
+        if !self.children.borrow().is_empty() {
+            if self.leak_children_on_drop.get() {
+                warn!("Dropping node with children outside of RcDom's destructor. \
+                       Leaking memory for {} children.", self.children.borrow().len());
+                for child in mem::replace(&mut *self.children.borrow_mut(), vec![]) {
+                    mem::forget(child);
+                }
+            }
+        }
+    }
 }
 
 impl fmt::Debug for Node {
@@ -195,6 +227,18 @@ pub struct RcDom {
     pub quirks_mode: QuirksMode,
 }
 
+impl Drop for RcDom {
+    fn drop(&mut self) {
+        // Ensure that node destructors execute linearly, rather
+        // than recursing through a tree of arbitrary depth.
+        let mut to_be_processed = vec![self.document.clone()];
+        while let Some(node) = to_be_processed.pop() {
+            to_be_processed.extend_from_slice(&*node.children.borrow());
+            node.children.borrow_mut().clear();
+        }
+    }
+}
+
 impl TreeSink for RcDom {
     type Output = Self;
     fn finish(self) -> Self {

xml5ever/examples/xml_tree_printer.rs

@@ -18,7 +18,7 @@ use xml5ever::driver::parse_document;
 use xml5ever::rcdom::{Handle, NodeData, RcDom};
 use xml5ever::tendril::TendrilSink;
 
-fn walk(prefix: &str, handle: Handle) {
+fn walk(prefix: &str, handle: &Handle) {
     let node = handle;
 
     print!("{}", prefix);
@@ -50,7 +50,7 @@ fn walk(prefix: &str, handle: Handle) {
             _ => false,
         })
     {
-        walk(&new_indent, child.clone());
+        walk(&new_indent, child);
     }
 }
 
@@ -69,5 +69,5 @@ fn main() {
         .unwrap();;
 
     // Execute our visualizer on RcDom
-    walk("", dom.document);
+    walk("", &dom.document);
 }