Bug 1397130 - Use signed integer for gUnusedAtomCount. r?froydnj
MozReview-Commit-ID: 9KweZdyu5WF
--- a/xpcom/ds/nsAtomTable.cpp
+++ b/xpcom/ds/nsAtomTable.cpp
@@ -62,17 +62,24 @@ class CheckStaticAtomSizes
(offsetof(nsFakeStringBuffer<1>, mStringData) ==
sizeof(nsStringBuffer)),
"mocked-up strings' representations should be compatible");
}
};
//----------------------------------------------------------------------
-static Atomic<uint32_t, ReleaseAcquire> gUnusedAtomCount(0);
+// gUnusedAtomCount is incremented when an atom loses its last reference
+// (and thus turned into unused state), and decremented when an unused
+// atom gets a reference again. The atom table relies on this value to
+// schedule GC. This value can temporarily go below zero when multiple
+// threads are operating the same atom, so it has to be signed so that
+// we wouldn't use overflow value for comparison.
+// See Atom::DynamicAddRef and Atom::DynamicRelease.
+static Atomic<int32_t, ReleaseAcquire> gUnusedAtomCount(0);
#if defined(NS_BUILD_REFCNT_LOGGING)
// nsFakeStringBuffers don't really use the refcounting system, but we
// have to give a coherent series of addrefs and releases to the
// refcount logging system, or we'll hit assertions when running with
// XPCOM_MEM_LOG_CLASSES=nsStringBuffer.
class FakeBufferRefcountHelper
{
@@ -401,17 +408,17 @@ Atom::GCAtomTable()
void
Atom::GCAtomTableLocked(const MutexAutoLock& aProofOfLock, GCKind aKind)
{
MOZ_ASSERT(NS_IsMainThread());
for (uint32_t i = 0; i < RECENTLY_USED_MAIN_THREAD_ATOM_CACHE_SIZE; ++i) {
sRecentlyUsedMainThreadAtoms[i] = nullptr;
}
- uint32_t removedCount = 0; // Use a non-atomic temporary for cheaper increments.
+ int32_t removedCount = 0; // Use a non-atomic temporary for cheaper increments.
nsAutoCString nonZeroRefcountAtoms;
uint32_t nonZeroRefcountAtomsCount = 0;
for (auto i = gAtomTable->Iter(); !i.Done(); i.Next()) {
auto entry = static_cast<AtomTableEntry*>(i.Get());
if (entry->mAtom->IsStaticAtom()) {
continue;
}
@@ -475,19 +482,19 @@ Atom::DynamicAddRef()
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.
-static const uint32_t kAtomGCThreshold = 20;
+static const int32_t kAtomGCThreshold = 20;
#else
-static const uint32_t kAtomGCThreshold = 10000;
+static const int32_t kAtomGCThreshold = 10000;
#endif
MozExternalRefCountType
Atom::DynamicRelease()
{
MOZ_ASSERT(IsDynamicAtom());
MOZ_ASSERT(mRefCnt > 0);
nsrefcnt count = --mRefCnt;
@@ -791,17 +798,17 @@ NS_AtomizeMainThread(const nsAString& aU
nsrefcnt
NS_GetNumberOfAtoms(void)
{
Atom::GCAtomTable(); // Trigger a GC so that we return a deterministic result.
MutexAutoLock lock(*gAtomTableLock);
return gAtomTable->EntryCount();
}
-uint32_t
+int32_t
NS_GetUnusedAtomCount(void)
{
return gUnusedAtomCount;
}
nsIAtom*
NS_GetStaticAtom(const nsAString& aUTF16String)
{
--- a/xpcom/tests/gtest/TestAtoms.cpp
+++ b/xpcom/tests/gtest/TestAtoms.cpp
@@ -12,17 +12,17 @@
#include "nsIServiceManager.h"
#include "nsStaticAtom.h"
#include "nsThreadUtils.h"
#include "gtest/gtest.h"
using namespace mozilla;
-uint32_t NS_GetUnusedAtomCount(void);
+int32_t NS_GetUnusedAtomCount(void);
namespace TestAtoms {
TEST(Atoms, Basic)
{
for (unsigned int i = 0; i < ArrayLength(ValidStrings); ++i) {
nsDependentString str16(ValidStrings[i].m16);
nsDependentCString str8(ValidStrings[i].m8);
@@ -172,22 +172,22 @@ private:
NS_IMPL_ISUPPORTS(nsAtomRunner, nsIRunnable)
TEST(Atoms, ConcurrentAccessing)
{
static const size_t kThreadCount = 4;
// Force a GC before so that we don't have any unused atom.
NS_GetNumberOfAtoms();
- EXPECT_EQ(NS_GetUnusedAtomCount(), uint32_t(0));
+ EXPECT_EQ(NS_GetUnusedAtomCount(), int32_t(0));
nsCOMPtr<nsIThread> threads[kThreadCount];
for (size_t i = 0; i < kThreadCount; i++) {
nsresult rv = NS_NewThread(getter_AddRefs(threads[i]), new nsAtomRunner);
EXPECT_TRUE(NS_SUCCEEDED(rv));
}
for (size_t i = 0; i < kThreadCount; i++) {
threads[i]->Shutdown();
}
// We should have one unused atom from this test.
- EXPECT_EQ(NS_GetUnusedAtomCount(), uint32_t(1));
+ EXPECT_EQ(NS_GetUnusedAtomCount(), int32_t(1));
}
}