From af44ad742c2b9510b6a424b74c7e5a25be78d7a9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Thu, 20 Feb 2020 08:43:08 +0000
Subject: [PATCH] Bug 1607553 - Remove an invalid assert. r=heycam

This assert was wrong. The assert may fire if we resurrect the node from a
different thread and insert a kid fast enough.

We allow resurrecting nodes (bumping the nodes from zero to one) to avoid
allocation churn.

In particular, while the thread dropping the node gets to read the children (so
after the fetch_sub from the refcount, but before the read() of the children),
another thread could plausibly bumped the refcount back, and added a children.

This is a very big edge case of course, but I'm kinda sad I hadn't realized
before.

Differential Revision: https://phabricator.services.mozilla.com/D63286

--HG--
extra : moz-landing-system : lando
---
 servo/components/style/rule_tree/mod.rs | 64 -------------------------
 1 file changed, 64 deletions(-)

diff --git a/servo/components/style/rule_tree/mod.rs b/servo/components/style/rule_tree/mod.rs
index 113fa9cb9bb5..ddac85a80750 100644
--- a/servo/components/style/rule_tree/mod.rs
+++ b/servo/components/style/rule_tree/mod.rs
@@ -136,29 +136,6 @@ impl StyleSource {
         let _ = write!(writer, "  -> {:?}", self.read(guard).declarations());
     }
 
-    // This is totally unsafe, should be removed when we figure out the cause of
-    // bug 1607553.
-    #[cfg(feature = "gecko")]
-    unsafe fn dump_unchecked<W: Write>(&self, writer: &mut W) {
-        if let Some(ref rule) = self.0.as_first() {
-            let rule = rule.read_unchecked();
-            let _ = write!(writer, "{:?}", rule.selectors);
-        }
-        let _ = write!(writer, "  -> {:?}", self.read_unchecked().declarations());
-    }
-
-    // This is totally unsafe, should be removed when we figure out the cause of
-    // bug 1607553.
-    #[inline]
-    #[cfg(feature = "gecko")]
-    unsafe fn read_unchecked(&self) -> &PropertyDeclarationBlock {
-        let block: &Locked<PropertyDeclarationBlock> = match self.0.borrow() {
-            ArcUnionBorrow::First(ref rule) => &rule.get().read_unchecked().block,
-            ArcUnionBorrow::Second(ref block) => block.get(),
-        };
-        block.read_unchecked()
-    }
-
     /// Read the style source guard, and obtain thus read access to the
     /// underlying property declaration block.
     #[inline]
@@ -1742,47 +1719,6 @@ impl Drop for StrongRuleNode {
             return;
         }
 
-        #[cfg(feature = "gecko")]
-        #[inline(always)]
-        fn assert_on_release() -> bool {
-            crate::gecko_bindings::structs::GECKO_IS_NIGHTLY
-        }
-
-        #[cfg(feature = "servo")]
-        fn assert_on_release() -> bool {
-            false
-        }
-
-        if cfg!(debug_assertions) || assert_on_release() {
-            let children = node.children.read();
-            if !children.is_empty() {
-                let mut crash_str = vec![];
-
-                #[cfg(feature = "gecko")]
-                unsafe {
-                    // Try to unsafely collect some information of this before
-                    // crashing the process.
-                    if let Some(ref s) = node.source {
-                        s.dump_unchecked(&mut crash_str);
-                        crash_str.push(b'\n');
-                    }
-                    children.each(|child| {
-                        (*child.ptr())
-                            .source
-                            .as_ref()
-                            .unwrap()
-                            .dump_unchecked(&mut crash_str);
-                        crash_str.push(b'\n');
-                    });
-                }
-
-                panic!(
-                    "Children left in the rule tree on drop: {}",
-                    String::from_utf8_lossy(&crash_str).trim()
-                );
-            }
-        }
-
         if node.parent.is_none() {
             debug!("Dropping root node!");
             // The free list should be null by this point