Bug 1187767 - Ensure PLDHashTable's generation is always updated when the entry store is modified. r=froydnj.

--HG--
extra : rebase_source : e70d253257eac422a454d20de0fb94d2ac736e84
This commit is contained in:
Nicholas Nethercote 2015-07-26 19:57:23 -07:00
parent 7b4828fcdf
commit dbc67380bd
2 changed files with 71 additions and 45 deletions

View File

@ -218,8 +218,7 @@ PLDHashTable::PLDHashTable(const PLDHashTableOps* aOps, uint32_t aEntrySize,
, mEntrySize(aEntrySize)
, mEntryCount(0)
, mRemovedCount(0)
, mGeneration(0)
, mEntryStore(nullptr)
, mEntryStore()
#ifdef DEBUG
, mChecker()
#endif
@ -248,7 +247,6 @@ PLDHashTable::operator=(PLDHashTable&& aOther)
mHashShift = Move(aOther.mHashShift);
mEntryCount = Move(aOther.mEntryCount);
mRemovedCount = Move(aOther.mRemovedCount);
mGeneration = Move(aOther.mGeneration);
mEntryStore = Move(aOther.mEntryStore);
#ifdef DEBUG
mChecker = Move(aOther.mChecker);
@ -259,7 +257,7 @@ PLDHashTable::operator=(PLDHashTable&& aOther)
#ifdef DEBUG
AutoDestructorOp op(mChecker);
#endif
aOther.mEntryStore = nullptr;
aOther.mEntryStore.Set(nullptr);
}
return *this;
@ -327,7 +325,8 @@ PLDHashTable::MatchEntryKeyhash(PLDHashEntryHdr* aEntry, PLDHashNumber aKeyHash)
PLDHashEntryHdr*
PLDHashTable::AddressEntry(uint32_t aIndex)
{
return reinterpret_cast<PLDHashEntryHdr*>(mEntryStore + aIndex * mEntrySize);
return reinterpret_cast<PLDHashEntryHdr*>(
mEntryStore.Get() + aIndex * mEntrySize);
}
PLDHashTable::~PLDHashTable()
@ -336,12 +335,12 @@ PLDHashTable::~PLDHashTable()
AutoDestructorOp op(mChecker);
#endif
if (!mEntryStore) {
if (!mEntryStore.Get()) {
return;
}
// Clear any remaining live entries.
char* entryAddr = mEntryStore;
char* entryAddr = mEntryStore.Get();
char* entryLimit = entryAddr + Capacity() * mEntrySize;
while (entryAddr < entryLimit) {
PLDHashEntryHdr* entry = (PLDHashEntryHdr*)entryAddr;
@ -351,9 +350,7 @@ PLDHashTable::~PLDHashTable()
entryAddr += mEntrySize;
}
// Free entry storage last.
free(mEntryStore);
mEntryStore = nullptr;
// Entry storage is freed last, by ~EntryStore().
}
void
@ -382,7 +379,7 @@ template <PLDHashTable::SearchReason Reason>
PLDHashEntryHdr* PL_DHASH_FASTCALL
PLDHashTable::SearchTable(const void* aKey, PLDHashNumber aKeyHash)
{
MOZ_ASSERT(mEntryStore);
MOZ_ASSERT(mEntryStore.Get());
NS_ASSERTION(!(aKeyHash & kCollisionFlag),
"!(aKeyHash & kCollisionFlag)");
@ -451,7 +448,7 @@ PLDHashTable::SearchTable(const void* aKey, PLDHashNumber aKeyHash)
PLDHashEntryHdr* PL_DHASH_FASTCALL
PLDHashTable::FindFreeEntry(PLDHashNumber aKeyHash)
{
MOZ_ASSERT(mEntryStore);
MOZ_ASSERT(mEntryStore.Get());
NS_ASSERTION(!(aKeyHash & kCollisionFlag),
"!(aKeyHash & kCollisionFlag)");
@ -490,7 +487,7 @@ PLDHashTable::FindFreeEntry(PLDHashNumber aKeyHash)
bool
PLDHashTable::ChangeTable(int32_t aDeltaLog2)
{
MOZ_ASSERT(mEntryStore);
MOZ_ASSERT(mEntryStore.Get());
// Look, but don't touch, until we succeed in getting new entry store.
int32_t oldLog2 = kHashBits - mHashShift;
@ -513,14 +510,13 @@ PLDHashTable::ChangeTable(int32_t aDeltaLog2)
// We can't fail from here on, so update table parameters.
mHashShift = kHashBits - newLog2;
mRemovedCount = 0;
mGeneration++;
// Assign the new entry store to table.
memset(newEntryStore, 0, nbytes);
char* oldEntryStore;
char* oldEntryAddr;
oldEntryAddr = oldEntryStore = mEntryStore;
mEntryStore = newEntryStore;
oldEntryAddr = oldEntryStore = mEntryStore.Get();
mEntryStore.Set(newEntryStore);
PLDHashMoveEntry moveEntry = mOps->moveEntry;
// Copy only live entries, leaving removed ones behind.
@ -544,7 +540,7 @@ PLDHashTable::ChangeTable(int32_t aDeltaLog2)
MOZ_ALWAYS_INLINE PLDHashNumber
PLDHashTable::ComputeKeyHash(const void* aKey)
{
MOZ_ASSERT(mEntryStore);
MOZ_ASSERT(mEntryStore.Get());
PLDHashNumber keyHash = mOps->hashKey(this, aKey);
keyHash *= kGoldenRatio;
@ -565,10 +561,10 @@ PLDHashTable::Search(const void* aKey)
AutoReadOp op(mChecker);
#endif
PLDHashEntryHdr* entry =
mEntryStore ? SearchTable<ForSearchOrRemove>(aKey, ComputeKeyHash(aKey))
: nullptr;
PLDHashEntryHdr* entry = mEntryStore.Get()
? SearchTable<ForSearchOrRemove>(aKey,
ComputeKeyHash(aKey))
: nullptr;
return entry;
}
@ -580,16 +576,16 @@ PLDHashTable::Add(const void* aKey, const mozilla::fallible_t&)
#endif
// Allocate the entry storage if it hasn't already been allocated.
if (!mEntryStore) {
if (!mEntryStore.Get()) {
uint32_t nbytes;
// We already checked this in the constructor, so it must still be true.
MOZ_RELEASE_ASSERT(SizeOfEntryStore(CapacityFromHashShift(), mEntrySize,
&nbytes));
mEntryStore = (char*)malloc(nbytes);
if (!mEntryStore) {
mEntryStore.Set((char*)malloc(nbytes));
if (!mEntryStore.Get()) {
return nullptr;
}
memset(mEntryStore, 0, nbytes);
memset(mEntryStore.Get(), 0, nbytes);
}
// If alpha is >= .75, grow or compress the table. If aKey is already in the
@ -638,7 +634,7 @@ PLDHashTable::Add(const void* aKey)
{
PLDHashEntryHdr* entry = Add(aKey, fallible);
if (!entry) {
if (!mEntryStore) {
if (!mEntryStore.Get()) {
// We OOM'd while allocating the initial entry storage.
uint32_t nbytes;
(void) SizeOfEntryStore(CapacityFromHashShift(), mEntrySize, &nbytes);
@ -661,9 +657,10 @@ PLDHashTable::Remove(const void* aKey)
AutoWriteOp op(mChecker);
#endif
PLDHashEntryHdr* entry =
mEntryStore ? SearchTable<ForSearchOrRemove>(aKey, ComputeKeyHash(aKey))
: nullptr;
PLDHashEntryHdr* entry = mEntryStore.Get()
? SearchTable<ForSearchOrRemove>(aKey,
ComputeKeyHash(aKey))
: nullptr;
if (entry) {
// Clear this entry and mark it as "removed".
RawRemove(entry);
@ -710,7 +707,7 @@ PLDHashTable::RawRemove(PLDHashEntryHdr* aEntry)
// active, which doesn't fit well into how Checker's mState variable works.
MOZ_ASSERT(mChecker.IsWritable());
MOZ_ASSERT(mEntryStore);
MOZ_ASSERT(mEntryStore.Get());
NS_ASSERTION(EntryIsLive(aEntry), "EntryIsLive(aEntry)");
@ -760,11 +757,11 @@ PLDHashTable::SizeOfExcludingThis(
AutoReadOp op(mChecker);
#endif
if (!mEntryStore) {
if (!mEntryStore.Get()) {
return 0;
}
size_t n = aMallocSizeOf(mEntryStore);
size_t n = aMallocSizeOf(mEntryStore.Get());
if (aSizeOfEntryExcludingThis) {
for (auto iter = ConstIter(); !iter.Done(); iter.Next()) {
n += aSizeOfEntryExcludingThis(iter.Get(), aMallocSizeOf, aArg);
@ -824,9 +821,9 @@ PLDHashTable::Iterator::Iterator(Iterator&& aOther)
PLDHashTable::Iterator::Iterator(PLDHashTable* aTable)
: mTable(aTable)
, mStart(mTable->mEntryStore)
, mLimit(mTable->mEntryStore + mTable->Capacity() * mTable->mEntrySize)
, mCurrent(mTable->mEntryStore)
, mStart(mTable->mEntryStore.Get())
, mLimit(mTable->mEntryStore.Get() + mTable->Capacity() * mTable->mEntrySize)
, mCurrent(mTable->mEntryStore.Get())
, mNexts(0)
, mNextsLimit(mTable->EntryCount())
, mHaveRemoved(false)

View File

@ -212,19 +212,48 @@ private:
// hashing is more space-efficient unless the element size gets large (in which
// case you should keep using double hashing but switch to using pointer
// elements). Also, with double hashing, you can't safely hold an entry pointer
// and use it after an ADD or REMOVE operation, unless you sample
// aTable->mGeneration before adding or removing, and compare the sample after,
// dereferencing the entry pointer only if aTable->mGeneration has not changed.
// and use it after an add or remove operation, unless you sample Generation()
// before adding or removing, and compare the sample after, dereferencing the
// entry pointer only if Generation() has not changed.
class PLDHashTable
{
private:
// This class maintains the invariant that every time the entry store is
// changed, the generation is updated.
class EntryStore
{
private:
char* mEntryStore;
uint32_t mGeneration;
public:
EntryStore() : mEntryStore(nullptr), mGeneration(0) {}
~EntryStore()
{
free(mEntryStore);
mEntryStore = nullptr;
mGeneration++; // a little paranoid, but why not be extra safe?
}
char* Get() { return mEntryStore; }
const char* Get() const { return mEntryStore; }
void Set(char* aEntryStore)
{
mEntryStore = aEntryStore;
mGeneration++;
}
uint32_t Generation() const { return mGeneration; }
};
const PLDHashTableOps* const mOps; // Virtual operations; see below.
int16_t mHashShift; // Multiplicative hash shift.
const uint32_t mEntrySize; // Number of bytes in an entry.
uint32_t mEntryCount; // Number of entries in table.
uint32_t mRemovedCount; // Removed entry sentinels in table.
uint32_t mGeneration; // Entry storage generation number.
char* mEntryStore; // Entry storage; allocated lazily.
EntryStore mEntryStore; // (Lazy) entry storage and generation.
#ifdef DEBUG
mutable Checker mChecker;
@ -264,9 +293,9 @@ public:
// move assignment operator cannot modify them.
: mOps(aOther.mOps)
, mEntrySize(aOther.mEntrySize)
// Initialize these two fields because they are required for a safe call
// to the destructor, which the move assignment operator does.
, mEntryStore(nullptr)
// Initialize this field because it is required for a safe call to the
// destructor, which the move assignment operator does.
, mEntryStore()
#ifdef DEBUG
, mChecker()
#endif
@ -286,12 +315,12 @@ public:
// entry storage will not have yet been allocated.
uint32_t Capacity() const
{
return mEntryStore ? CapacityFromHashShift() : 0;
return mEntryStore.Get() ? CapacityFromHashShift() : 0;
}
uint32_t EntrySize() const { return mEntrySize; }
uint32_t EntryCount() const { return mEntryCount; }
uint32_t Generation() const { return mGeneration; }
uint32_t Generation() const { return mEntryStore.Generation(); }
// To search for a |key| in |table|, call:
//