Bug 1381731 - Remove gUnusedAtomCount assertions since they're hard to get right given OMT atom refcounting. r?bholley
MozReview-Commit-ID: ByM6mzL1IgF
--- a/xpcom/ds/nsAtomTable.cpp
+++ b/xpcom/ds/nsAtomTable.cpp
@@ -458,43 +458,42 @@ DynamicAtom::GCAtomTableLocked(const Mut
}
if (nonZeroRefcountAtomsCount) {
nsPrintfCString msg("%d dynamic atom(s) with non-zero refcount: %s",
nonZeroRefcountAtomsCount, nonZeroRefcountAtoms.get());
NS_ASSERTION(nonZeroRefcountAtomsCount == 0, msg.get());
}
- // During the course of this function, the atom table is locked. This means
- // that, barring refcounting bugs in consumers, an atom can never go from
- // refcount == 0 to refcount != 0 during a GC. However, an atom _can_ go from
- // refcount != 0 to refcount == 0 if a Release() occurs in parallel with GC.
- // This means that we cannot assert that gUnusedAtomCount == removedCount, and
- // thus that there are no unused atoms at the end of a GC. We can and do,
- // however, assert this after the last GC at shutdown.
- if (aKind == GCKind::RegularOperation) {
- MOZ_ASSERT(removedCount <= gUnusedAtomCount);
- } else {
- // Complain if somebody adds new GCKind enums.
- MOZ_ASSERT(aKind == GCKind::Shutdown);
- // Our unused atom count should be accurate.
- MOZ_ASSERT(removedCount == gUnusedAtomCount);
- }
+ // We would like to assert that gUnusedAtomCount matches the number of atoms
+ // we found in the table which we removed. During the course of this function,
+ // the atom table is locked, but this lock is not acquired for AddRef() and
+ // Release() calls. This means we might see a gUnusedAtomCount value in
+ // between, say, AddRef() incrementing mRefCnt and it decrementing
+ // gUnusedAtomCount. So, we don't bother asserting that there are no unused
+ // atoms at the end of a regular GC. But we can (and do) assert thist just
+ // after the last GC at shutdown.
+ //
+ // Note that, barring refcounting bugs, an atom can only go from a zero
+ // refcount to a non-zero refcount while the atom table lock is held, so
+ // so we won't try to resurrect a zero refcount atom while trying to delete
+ // it.
+
+ MOZ_ASSERT_IF(aKind == GCKind::Shutdown, removedCount == gUnusedAtomCount);
gUnusedAtomCount -= removedCount;
}
NS_IMPL_QUERY_INTERFACE(DynamicAtom, nsIAtom)
NS_IMETHODIMP_(MozExternalRefCountType)
DynamicAtom::AddRef(void)
{
nsrefcnt count = ++mRefCnt;
if (count == 1) {
- MOZ_ASSERT(gUnusedAtomCount > 0);
gUnusedAtomCount--;
}
return count;
}
#ifdef DEBUG
// We set a lower GC threshold for atoms in debug builds so that we exercise
// the GC machinery more often.