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 9278714d3c9b4ca2f9ad753ac2cd0de78dc8fddb..08b7aa824bca1a633ff53bcba19345bebfedff8c 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 3397d60f5f752e4976fefd82b7e96fd27947b497..7d83694d92317fa91b1bbc0b75ba3a117b8c55ec 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 de67513cefdf3ca82d542b38109f1060c1670bc0..7da41f67300fcc2474efb42982a027dc52da6dbc 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 a64fed441504a599abc2a7177f2b7f495b3ce26b..c49716d368dc31988f2e832f7ac506c7b987d34c 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>