From 48aa7070acc303e7befe4975fa6dfb1836763b9e Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Fri, 18 Nov 2016 16:59:24 +0800 Subject: [PATCH 1/3] GC the RuleTree when the Stylist is dropped. We only consider doing a GC currently if the root node has a zero refcount. But that only happens if it has no children -- even weak children keep a strong reference to their parent. So at the very least, we should do a GC specifically when the RuleTree is going away. (We probably want to add some other GC opportunities too at some point, otherwise it's easy to never GC a RuleTree.) --- components/style/stylist.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index de106e7a3de..83bce4bec58 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -624,6 +624,18 @@ impl Stylist { } } +impl Drop for Stylist { + fn drop(&mut self) { + // This is the last chance to GC the rule tree. If we have dropped all + // strong rule node references before the Stylist is dropped, then this + // will cause the rule tree to be destroyed correctly. If we haven't + // dropped all strong rule node references before now, then we will + // leak them, since there will be no way to call gc() on the rule tree + // after this point. + unsafe { self.rule_tree.gc(); } + } +} + /// Map that contains the CSS rules for a specific PseudoElement /// (or lack of PseudoElement). From 226c946817465a83dd22c96a0d4cd6733d536caa Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Fri, 18 Nov 2016 17:01:05 +0800 Subject: [PATCH 2/3] Mark the Gecko main thread as a Servo layout thread. This is needed to keep the assertion in the RuleTree GC functions that checks we're being called on the non-worker layout thread. --- ports/geckolib/glue.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 37a69ed07c8..c0b4d59d373 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -46,6 +46,7 @@ use style::selector_parser::PseudoElementCascadeType; use style::sequential; use style::string_cache::Atom; use style::stylesheets::{Origin, Stylesheet}; +use style::thread_state; use style::timer::Timer; use style_traits::ToCss; @@ -66,6 +67,9 @@ pub extern "C" fn Servo_Initialize() -> () { // Allocate our default computed values. unsafe { ComputedValues::initialize(); } + + // Pretend that we're a Servo Layout thread, to make some assertions happy. + thread_state::initialize(thread_state::LAYOUT); } #[no_mangle] From d92cc8ecd02bffb312f529c7dd90f6c1bc79ff40 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Mon, 21 Nov 2016 17:04:58 +0800 Subject: [PATCH 3/3] Don't assert when we do a rule tree GC and the root nodes still has strong references. It could still have children. --- components/style/rule_tree/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index 2e87a30c212..1e9602428c0 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -440,8 +440,7 @@ impl StrongRuleNode { // script. debug_assert!(!thread_state::get().is_worker() && (thread_state::get().is_layout() || - (thread_state::get().is_script() && - me.refcount.load(Ordering::SeqCst) == 0))); + thread_state::get().is_script())); let current = me.next_free.load(Ordering::SeqCst); if current == FREE_LIST_SENTINEL {