aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorbors-servo <lbergstrom+bors@mozilla.com>2016-11-11 13:33:06 -0600
committerGitHub <noreply@github.com>2016-11-11 13:33:06 -0600
commitd49840eeec30967cc6f482bc2db9055ab6404ea4 (patch)
tree70230eb15dcd5b574fd09e956c4d19dec58c8701
parent75d35241db63894db31b6385143ffea0db5cac70 (diff)
parent65f239c9e11cb8a1a95477560c3ef9400c924110 (diff)
downloadservo-d49840eeec30967cc6f482bc2db9055ab6404ea4.tar.gz
servo-d49840eeec30967cc6f482bc2db9055ab6404ea4.zip
Auto merge of #14167 - emilio:rule-tree-list, r=Manishearth
style: Don't assume siblings are alive in the rule tree when removing ourselves from the child list. <!-- 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: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors <!-- Either: --> - [x] There are tests for these changes OR <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> We can't assume all our siblings are alive because they may very well be in the free list too. This tempts to happen when the rule nodes are destroyed as part of the last GC, the one that runs in the root destructor. Also, properly put the next sibling back into the list when the rules are GCd. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14167) <!-- Reviewable:end -->
-rw-r--r--components/style/rule_tree/mod.rs27
1 files changed, 16 insertions, 11 deletions
diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs
index 5534d76323a..f1574ea5856 100644
--- a/components/style/rule_tree/mod.rs
+++ b/components/style/rule_tree/mod.rs
@@ -200,21 +200,26 @@ impl RuleNode {
unsafe fn remove_from_child_list(&self) {
debug!("Remove from child list: {:?}, parent: {:?}",
self as *const RuleNode, self.parent.as_ref().map(|p| p.ptr()));
- let prev_sibling = self.prev_sibling.swap(ptr::null_mut(), Ordering::SeqCst);
- let next_sibling = self.next_sibling.swap(ptr::null_mut(), Ordering::SeqCst);
-
- if prev_sibling != ptr::null_mut() {
- let really_previous = WeakRuleNode { ptr: prev_sibling };
- really_previous.upgrade()
- .get().next_sibling.store(next_sibling, Ordering::SeqCst);
+ // NB: The other siblings we use in this function can also be dead, so
+ // we can't use `get` here, since it asserts.
+ let prev_sibling = self.prev_sibling.swap(ptr::null_mut(), Ordering::Relaxed);
+ let next_sibling = self.next_sibling.swap(ptr::null_mut(), Ordering::Relaxed);
+
+ // Store the `next` pointer as appropriate, either in the previous
+ // sibling, or in the parent otherwise.
+ if prev_sibling == ptr::null_mut() {
+ let parent = self.parent.as_ref().unwrap();
+ parent.get().first_child.store(next_sibling, Ordering::Relaxed);
} else {
- self.parent.as_ref().unwrap()
- .get().first_child.store(ptr::null_mut(), Ordering::SeqCst);
+ let previous = &*prev_sibling;
+ previous.next_sibling.store(next_sibling, Ordering::Relaxed);
}
+ // Store the previous sibling pointer in the next sibling if present,
+ // otherwise we're done.
if next_sibling != ptr::null_mut() {
- let really_next = WeakRuleNode { ptr: next_sibling };
- really_next.upgrade().get().prev_sibling.store(prev_sibling, Ordering::SeqCst);
+ let next = &*next_sibling;
+ next.prev_sibling.store(prev_sibling, Ordering::Relaxed);
}
}