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
This commit is contained in:
Emilio Cobos Álvarez 2020-02-20 08:43:08 +00:00
Родитель 21292e4991
Коммит af44ad742c
1 изменённых файлов: 0 добавлений и 64 удалений

Просмотреть файл

@ -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