Fix <rdar://5578982> ASSERT in HashTable::checkTableConsistencyExceptSize...

        The bug was due to a mismatch between HashMap::remove and
        HashTable::checkTableConsistency. HashMap::remove can delete the value
        stored in the HashTable (by derefing it), which is not normally
        allowed by HashTable. It's OK in this case because the value is about
        to be removed from the table, but HashTable wasn't aware of this.

        HashMap::remove now performs the consistency check itself before
        derefing the value.

        Darin noticed that the same bug would occur in HashSet, so I've fixed
        it there as well.

        Reviewed by Darin.

        * wtf/HashMap.h:
        (WTF::HashMap::remove): Perform the HashTable consistency check
        manually before calling deref.
        * wtf/HashSet.h:
        (WTF::HashSet::remove): Ditto.
        * wtf/HashTable.h: Made checkTableConsistency public so that HashMap
        and HashSet can call it.
        (WTF::HashTable::removeAndInvalidateWithoutEntryConsistencyCheck):
        Added.
        (WTF::HashTable::removeAndInvalidate): Added.
        (WTF::HashTable::remove):
        (WTF::HashTable::removeWithoutEntryConsistencyCheck): Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@27710 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 9278714..08b7aa8 100644 (file)
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,34 @@
+2007-11-11  Adam Roben  <aroben@apple.com>
+
+        Fix <rdar://5578982> ASSERT in HashTable::checkTableConsistencyExceptSize beneath WebNotificationCenter
+
+        The bug was due to a mismatch between HashMap::remove and
+        HashTable::checkTableConsistency. HashMap::remove can delete the value
+        stored in the HashTable (by derefing it), which is not normally
+        allowed by HashTable. It's OK in this case because the value is about
+        to be removed from the table, but HashTable wasn't aware of this.
+
+        HashMap::remove now performs the consistency check itself before
+        derefing the value.
+
+        Darin noticed that the same bug would occur in HashSet, so I've fixed
+        it there as well.
+
+        Reviewed by Darin.
+
+        * wtf/HashMap.h:
+        (WTF::HashMap::remove): Perform the HashTable consistency check
+        manually before calling deref.
+        * wtf/HashSet.h:
+        (WTF::HashSet::remove): Ditto.
+        * wtf/HashTable.h: Made checkTableConsistency public so that HashMap
+        and HashSet can call it.
+        (WTF::HashTable::removeAndInvalidateWithoutEntryConsistencyCheck):
+        Added.
+        (WTF::HashTable::removeAndInvalidate): Added.
+        (WTF::HashTable::remove):
+        (WTF::HashTable::removeWithoutEntryConsistencyCheck): Added.
+
 2007-11-11  Mark Rowe  <mrowe@apple.com>
 
         Build fix.  Use the correct filename case.

diff --git a/JavaScriptCore/wtf/HashMap.h b/JavaScriptCore/wtf/HashMap.h
index 3397d60..7d83694 100644 (file)
--- a/JavaScriptCore/wtf/HashMap.h
+++ b/JavaScriptCore/wtf/HashMap.h
@@ -307,8 +307,9 @@ namespace WTF {
     {
         if (it.m_impl == m_impl.end())
             return;
+        m_impl.checkTableConsistency();
         RefCounter<ValueTraits, ValueStorageTraits>::deref(*it.m_impl);
-        m_impl.remove(it.m_impl);
+        m_impl.removeWithoutEntryConsistencyCheck(it.m_impl);
     }
 
     template<typename T, typename U, typename V, typename W, typename X>

diff --git a/JavaScriptCore/wtf/HashSet.h b/JavaScriptCore/wtf/HashSet.h
index de67513..7da41f6 100644 (file)
--- a/JavaScriptCore/wtf/HashSet.h
+++ b/JavaScriptCore/wtf/HashSet.h
@@ -284,8 +284,9 @@ namespace WTF {
     {
         if (it.m_impl == m_impl.end())
             return;
+        m_impl.checkTableConsistency();
         RefCounter<ValueTraits, StorageTraits>::deref(*it.m_impl);
-        m_impl.remove(it.m_impl);
+        m_impl.removeWithoutEntryConsistencyCheck(it.m_impl);
     }
 
     template<typename T, typename U, typename V>

diff --git a/JavaScriptCore/wtf/HashTable.h b/JavaScriptCore/wtf/HashTable.h
index a64fed4..c49716d 100644 (file)
--- a/JavaScriptCore/wtf/HashTable.h
+++ b/JavaScriptCore/wtf/HashTable.h
@@ -321,6 +321,7 @@ namespace WTF {
 
         void remove(const KeyType&);
         void remove(iterator);
+        void removeWithoutEntryConsistencyCheck(iterator);
         void clear();
 
         static bool isEmptyBucket(const ValueType& value) { return Extractor::extract(value) == KeyTraits::emptyValue(); }
@@ -329,6 +330,12 @@ namespace WTF {
 
         ValueType* lookup(const Key& key) { return lookup<Key, IdentityTranslatorType>(key); }
 
+#if CHECK_HASHTABLE_CONSISTENCY
+        void checkTableConsistency() const;
+#else
+        static void checkTableConsistency() { }
+#endif
+
     private:
         static ValueType* allocateTable(int size);
         static void deallocateTable(ValueType* table, int size);
@@ -341,6 +348,8 @@ namespace WTF {
         template<typename T, typename HashTranslator> FullLookupType fullLookupForWriting(const T&);
         template<typename T, typename HashTranslator> LookupType lookupForWriting(const T&);
 
+        void removeAndInvalidateWithoutEntryConsistencyCheck(ValueType*);
+        void removeAndInvalidate(ValueType*);
         void remove(ValueType*);
 
         bool shouldExpand() const { return (m_keyCount + m_deletedCount) * m_maxLoad >= m_tableSize; }
@@ -364,10 +373,8 @@ namespace WTF {
         const_iterator makeKnownGoodConstIterator(ValueType* pos) const { return const_iterator(this, pos, m_table + m_tableSize, HashItemKnownGood); }
 
 #if CHECK_HASHTABLE_CONSISTENCY
-        void checkTableConsistency() const;
         void checkTableConsistencyExceptSize() const;
 #else
-        static void checkTableConsistency() { }
         static void checkTableConsistencyExceptSize() { }
 #endif
 
@@ -758,11 +765,23 @@ namespace WTF {
     }
 
     template<typename Key, typename Value, typename Extractor, typename HashFunctions, typename Traits, typename KeyTraits>
-    void HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::remove(ValueType* pos)
+    void HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::removeAndInvalidateWithoutEntryConsistencyCheck(ValueType* pos)
+    {
+        invalidateIterators();
+        remove(pos);
+    }
+
+    template<typename Key, typename Value, typename Extractor, typename HashFunctions, typename Traits, typename KeyTraits>
+    void HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::removeAndInvalidate(ValueType* pos)
     {
         invalidateIterators();
         checkTableConsistency();
+        remove(pos);
+    }
 
+    template<typename Key, typename Value, typename Extractor, typename HashFunctions, typename Traits, typename KeyTraits>
+    void HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::remove(ValueType* pos)
+    {
 #if DUMP_HASHTABLE_STATS
         ++HashTableStats::numRemoves;
 #endif
@@ -783,7 +802,16 @@ namespace WTF {
         if (it == end())
             return;
 
-        remove(const_cast<ValueType*>(it.m_iterator.m_position));
+        removeAndInvalidate(const_cast<ValueType*>(it.m_iterator.m_position));
+    }
+
+    template<typename Key, typename Value, typename Extractor, typename HashFunctions, typename Traits, typename KeyTraits>
+    inline void HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::removeWithoutEntryConsistencyCheck(iterator it)
+    {
+        if (it == end())
+            return;
+
+        removeAndInvalidateWithoutEntryConsistencyCheck(const_cast<ValueType*>(it.m_iterator.m_position));
     }
 
     template<typename Key, typename Value, typename Extractor, typename HashFunctions, typename Traits, typename KeyTraits>