Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure all RcDom nodes drop children iteratively #384

Merged
merged 2 commits into from
Jul 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions html5ever/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]

name = "html5ever"
version = "0.23.0"
version = "0.24.0"
authors = [ "The html5ever Project Developers" ]
license = "MIT / Apache-2.0"
repository = "https://github.com/servo/html5ever"
Expand Down Expand Up @@ -29,7 +29,7 @@ name = "serializer"
[dependencies]
log = "0.4"
mac = "0.1"
markup5ever = { version = "0.8", path = "../markup5ever" }
markup5ever = { version = "0.9", path = "../markup5ever" }

[dev-dependencies]
serde_json = "1.0"
Expand Down
2 changes: 1 addition & 1 deletion markup5ever/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "markup5ever"
version = "0.8.1"
version = "0.9.0"
authors = [ "The html5ever Project Developers" ]
license = "MIT / Apache-2.0"
repository = "https://github.com/servo/html5ever"
Expand Down
42 changes: 4 additions & 38 deletions markup5ever/rcdom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,6 @@ 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 {
Expand All @@ -120,36 +118,16 @@ 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);
}
}
let mut nodes = mem::replace(&mut *self.children.borrow_mut(), vec![]);
while let Some(node) = nodes.pop() {
let children = mem::replace(&mut *node.children.borrow_mut(), vec![]);
nodes.extend(children.into_iter());
}
}
}
Expand Down Expand Up @@ -227,18 +205,6 @@ 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 {
Expand Down
4 changes: 2 additions & 2 deletions xml5ever/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]

name = "xml5ever"
version = "0.14.0"
version = "0.15.0"
authors = ["The xml5ever project developers"]
license = "MIT / Apache-2.0"
repository = "https://github.com/servo/html5ever"
Expand All @@ -23,7 +23,7 @@ doctest = true
time = "0.1"
log = "0.4"
mac = "0.1"
markup5ever = {version = "0.8", path = "../markup5ever" }
markup5ever = {version = "0.9", path = "../markup5ever" }

[dev-dependencies]
serde_json = "1.0"
Expand Down