Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-21 01:05:45 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

servo: Merge #17563 - Bug 1368302 - stylo: possible stack overflow when processing very dee… (from julian-seward1:master); r=bholley

Browse Source 
…p DOM.  r=bholley.

Whilst working with variants of the bloom-basic test for Stylo perf
profiling, I noticed that it is easy to cause Firefox to segfault on DOMs
with a depth of more than about 1500 elements.  This fix limits the use of
tail recursion to 150 elements.  This isn't externally observable to content
-- we're still completely correct, just not using tail recursion any more.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 683a53b080fa119562f6d4ee3a4deb0cc863c128

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 01ea54b32f447d0890b1effbe439a5b56f627232
This commit is contained in:
Julian Seward 2017-06-30 03:01:08 -07:00
parent 4b64b92ce3
commit 7a6431b33b
1 changed files with 23 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
servo/components/style/parallel.rs
View File

@ -92,6 +92,7 @@ pub fn traverse_dom<E, D>(traversal: &D,
let nodes = nodes;
traverse_nodes(&*nodes,
DispatchMode::TailCall,
0,
root,
traversal_data,
scope,
@ -145,6 +146,7 @@ fn create_thread_local_context<'scope, E, D>(
#[inline(always)]
#[allow(unsafe_code)]
fn top_down_dom<'a, 'scope, E, D>(nodes: &'a [SendNode<E::ConcreteNode>],
recursion_depth: usize,
root: OpaqueNode,
mut traversal_data: PerLevelTraversalData,
scope: &'a rayon::Scope<'scope>,
@ -186,6 +188,7 @@ fn top_down_dom<'a, 'scope, E, D>(nodes: &'a [SendNode<E::ConcreteNode>],
traversal_data_copy.current_dom_depth += 1;
traverse_nodes(&*children,
DispatchMode::NotTailCall,
recursion_depth,
root,
traversal_data_copy,
scope,
@ -216,6 +219,7 @@ fn top_down_dom<'a, 'scope, E, D>(nodes: &'a [SendNode<E::ConcreteNode>],
traversal_data.current_dom_depth += 1;
traverse_nodes(&discovered_child_nodes,
DispatchMode::TailCall,
recursion_depth,
root,
traversal_data,
scope,
@ -237,9 +241,16 @@ impl DispatchMode {
fn is_tail_call(&self) -> bool { matches!(*self, DispatchMode::TailCall) }
}
// On x86_64-linux, a recursive cycle requires 3472 bytes of stack. Limiting
// the depth to 150 therefore should keep the stack use by the recursion to
// 520800 bytes, which would give a generously conservative margin should we
// decide to reduce the thread stack size from its default of 2MB down to 1MB.
const RECURSION_DEPTH_LIMIT: usize = 150;
#[inline]
fn traverse_nodes<'a, 'scope, E, D>(nodes: &[SendNode<E::ConcreteNode>],
mode: DispatchMode,
recursion_depth: usize,
root: OpaqueNode,
traversal_data: PerLevelTraversalData,
scope: &'a rayon::Scope<'scope>,
@ -254,8 +265,13 @@ fn traverse_nodes<'a, 'scope, E, D>(nodes: &[SendNode<E::ConcreteNode>],
// This is a tail call from the perspective of the caller. However, we only
// want to actually dispatch the job as a tail call if there's nothing left
// in our local queue. Otherwise we need to return to it to maintain proper
// breadth-first ordering.
// breadth-first ordering. We also need to take care to avoid stack
// overflow due to excessive tail recursion. The stack overflow isn't
// observable to content -- we're still completely correct, just not
// using tail recursion any more. See bug 1368302.
debug_assert!(recursion_depth <= RECURSION_DEPTH_LIMIT);
let may_dispatch_tail = mode.is_tail_call() &&
recursion_depth != RECURSION_DEPTH_LIMIT &&
!pool.current_thread_has_pending_tasks().unwrap();
// In the common case, our children fit within a single work unit, in which
@ -263,11 +279,13 @@ fn traverse_nodes<'a, 'scope, E, D>(nodes: &[SendNode<E::ConcreteNode>],
if nodes.len() <= WORK_UNIT_MAX {
let work = nodes.iter().cloned().collect::<WorkUnit<E::ConcreteNode>>();
if may_dispatch_tail {
top_down_dom(&work, root, traversal_data, scope, pool, traversal, tls);
top_down_dom(&work, recursion_depth + 1, root,
traversal_data, scope, pool, traversal, tls);
} else {
scope.spawn(move |scope| {
let work = work;
top_down_dom(&work, root, traversal_data, scope, pool, traversal, tls);
top_down_dom(&work, 0, root,
traversal_data, scope, pool, traversal, tls);
});
}
} else {
@ -276,7 +294,8 @@ fn traverse_nodes<'a, 'scope, E, D>(nodes: &[SendNode<E::ConcreteNode>],
let traversal_data_copy = traversal_data.clone();
scope.spawn(move |scope| {
let n = nodes;
top_down_dom(&*n, root, traversal_data_copy, scope, pool, traversal, tls)
top_down_dom(&*n, 0, root,
traversal_data_copy, scope, pool, traversal, tls)
});
}
}