Add ability to mark JSDHashTable/PLDHashTable as immutable and thus prevent RECURSION_LEVEL assertions from firing due to lookups racing on multiple threads. (Bug 469004) r=brendan,mrbkap

js/src/jsdhash.cpp

@@ -71,16 +71,32 @@
 #define RECURSION_LEVEL(table_) (*(uint32*)(table_->entryStore + \
                                             JS_DHASH_TABLE_SIZE(table_) * \
                                             table_->entrySize))
+/*
+ * Most callers that assert about the recursion level don't care about
+ * this magical value because they are asserting that mutation is
+ * allowed (and therefore the level is 0 or 1, depending on whether they
+ * incremented it).
+ *
+ * Only PL_DHashTableFinish needs to allow this special value.
+ */
+#define IMMUTABLE_RECURSION_LEVEL ((uint32)-1)
+
+#define RECURSION_LEVEL_SAFE_TO_FINISH(table_)                                \
+    (RECURSION_LEVEL(table_) == 0 ||                                          \
+     RECURSION_LEVEL(table_) == IMMUTABLE_RECURSION_LEVEL)
 
 #define ENTRY_STORE_EXTRA                   sizeof(uint32)
-#define INCREMENT_RECURSION_LEVEL(table_)   \
-    JS_BEGIN_MACRO                          \
-      ++RECURSION_LEVEL(table_);            \
+#define INCREMENT_RECURSION_LEVEL(table_)                                     \
+    JS_BEGIN_MACRO                                                            \
+        if (RECURSION_LEVEL(table_) != IMMUTABLE_RECURSION_LEVEL)             \
+            ++RECURSION_LEVEL(table_);                                        \
     JS_END_MACRO
-#define DECREMENT_RECURSION_LEVEL(table_)                  \
-    JS_BEGIN_MACRO                                         \
-      JSDHASH_ONELINE_ASSERT(RECURSION_LEVEL(table_) > 0); \
-      --RECURSION_LEVEL(table_);                           \
+#define DECREMENT_RECURSION_LEVEL(table_)                                     \
+    JS_BEGIN_MACRO                                                            \
+        if (RECURSION_LEVEL(table_) != IMMUTABLE_RECURSION_LEVEL) {           \
+            JSDHASH_ONELINE_ASSERT(RECURSION_LEVEL(table_) > 0);              \
+            --RECURSION_LEVEL(table_);                                        \
+        }                                                                     \
     JS_END_MACRO
 
 #else
@@ -382,7 +398,7 @@ JS_DHashTableFinish(JSDHashTable *table)
     }
 
     DECREMENT_RECURSION_LEVEL(table);
-    JS_ASSERT(RECURSION_LEVEL(table) == 0);
+    JS_ASSERT(RECURSION_LEVEL_SAFE_TO_FINISH(table));
 
     /* Free entry storage last. */
     table->ops->freeTable(table, table->entryStore);
@@ -688,6 +704,8 @@ JS_DHashTableRawRemove(JSDHashTable *table, JSDHashEntryHdr *entry)
 {
     JSDHashNumber keyHash;      /* load first in case clearEntry goofs it */
 
+    JS_ASSERT(RECURSION_LEVEL(table) != IMMUTABLE_RECURSION_LEVEL);
+
     JS_ASSERT(JS_DHASH_ENTRY_IS_LIVE(entry));
     keyHash = entry->keyHash;
     table->ops->clearEntry(table, entry);
@@ -763,6 +781,14 @@ JS_DHashTableEnumerate(JSDHashTable *table, JSDHashEnumerator etor, void *arg)
     return i;
 }
 
+#ifdef DEBUG
+JS_PUBLIC_API(void)
+JS_DHashMarkTableImmutable(JSDHashTable *table)
+{
+    RECURSION_LEVEL(table) = IMMUTABLE_RECURSION_LEVEL;
+}
+#endif
+
 #ifdef JS_DHASHMETER
 #include <math.h>
 

js/src/jsdhash.h

@@ -576,6 +576,25 @@ typedef JSDHashOperator
 extern JS_PUBLIC_API(uint32)
 JS_DHashTableEnumerate(JSDHashTable *table, JSDHashEnumerator etor, void *arg);
 
+#ifdef DEBUG
+/**
+ * Mark a table as immutable for the remainder of its lifetime.  This
+ * changes the implementation from ASSERTing one set of invariants to
+ * ASSERTing a different set.
+ *
+ * When a table is NOT marked as immutable, the table implementation
+ * asserts that the table is not mutated from its own callbacks.  It
+ * assumes the caller protects the table from being accessed on multiple
+ * threads simultaneously.
+ *
+ * When the table is marked as immutable, the re-entry assertions will
+ * no longer trigger erroneously due to multi-threaded access.  Instead,
+ * mutations will cause assertions.
+ */
+extern JS_PUBLIC_API(void)
+JS_DHashMarkTableImmutable(JSDHashTable *table);
+#endif
+
 #ifdef JS_DHASHMETER
 #include <stdio.h>
 

parser/htmlparser/src/nsHTMLEntities.cpp

@@ -156,6 +156,10 @@ nsHTMLEntities::AddRefTable(void)
       if (!entry->node)
         entry->node = node;
     }
+#ifdef DEBUG
+    PL_DHashMarkTableImmutable(&gUnicodeToEntity);
+    PL_DHashMarkTableImmutable(&gEntityToUnicode);
+#endif
   }
   ++gTableRefCnt;
   return NS_OK;

xpcom/glue/pldhash.c

@@ -72,16 +72,32 @@
 #define RECURSION_LEVEL(table_) (*(PRUint32*)(table_->entryStore + \
                                             PL_DHASH_TABLE_SIZE(table_) * \
                                             table_->entrySize))
+/*
+ * Most callers that assert about the recursion level don't care about
+ * this magical value because they are asserting that mutation is
+ * allowed (and therefore the level is 0 or 1, depending on whether they
+ * incremented it).
+ *
+ * Only PL_DHashTableFinish needs to allow this special value.
+ */
+#define IMMUTABLE_RECURSION_LEVEL ((PRUint32)-1)
+
+#define RECURSION_LEVEL_SAFE_TO_FINISH(table_)                                \
+    (RECURSION_LEVEL(table_) == 0 ||                                          \
+     RECURSION_LEVEL(table_) == IMMUTABLE_RECURSION_LEVEL)
 
 #define ENTRY_STORE_EXTRA                   sizeof(PRUint32)
-#define INCREMENT_RECURSION_LEVEL(table_)   \
-    PR_BEGIN_MACRO                          \
-      ++RECURSION_LEVEL(table_);            \
+#define INCREMENT_RECURSION_LEVEL(table_)                                     \
+    PR_BEGIN_MACRO                                                            \
+        if (RECURSION_LEVEL(table_) != IMMUTABLE_RECURSION_LEVEL)             \
+            ++RECURSION_LEVEL(table_);                                        \
     PR_END_MACRO
-#define DECREMENT_RECURSION_LEVEL(table_)                  \
-    PR_BEGIN_MACRO                                         \
-      NS_ASSERTION(RECURSION_LEVEL(table_) > 0, "RECURSION_LEVEL(table_) > 0"); \
-      --RECURSION_LEVEL(table_);                           \
+#define DECREMENT_RECURSION_LEVEL(table_)                                     \
+    PR_BEGIN_MACRO                                                            \
+        if (RECURSION_LEVEL(table_) != IMMUTABLE_RECURSION_LEVEL) {           \
+            NS_ASSERTION(RECURSION_LEVEL(table_) > 0, "RECURSION_LEVEL(table_) > 0");              \
+            --RECURSION_LEVEL(table_);                                        \
+        }                                                                     \
     PR_END_MACRO
 
 #else
@@ -386,8 +402,8 @@ PL_DHashTableFinish(PLDHashTable *table)
     }
 
     DECREMENT_RECURSION_LEVEL(table);
-    NS_ASSERTION(RECURSION_LEVEL(table) == 0,
-                 "RECURSION_LEVEL(table) == 0");
+    NS_ASSERTION(RECURSION_LEVEL_SAFE_TO_FINISH(table),
+                 "RECURSION_LEVEL_SAFE_TO_FINISH(table)");
 
     /* Free entry storage last. */
     table->ops->freeTable(table, table->entryStore);
@@ -698,6 +714,9 @@ PL_DHashTableRawRemove(PLDHashTable *table, PLDHashEntryHdr *entry)
 {
     PLDHashNumber keyHash;      /* load first in case clearEntry goofs it */
 
+    NS_ASSERTION(RECURSION_LEVEL(table) != IMMUTABLE_RECURSION_LEVEL,
+                 "RECURSION_LEVEL(table) != IMMUTABLE_RECURSION_LEVEL");
+
     NS_ASSERTION(PL_DHASH_ENTRY_IS_LIVE(entry),
                  "PL_DHASH_ENTRY_IS_LIVE(entry)");
     keyHash = entry->keyHash;
@@ -775,6 +794,14 @@ PL_DHashTableEnumerate(PLDHashTable *table, PLDHashEnumerator etor, void *arg)
     return i;
 }
 
+#ifdef DEBUG
+void
+PL_DHashMarkTableImmutable(PLDHashTable *table)
+{
+    RECURSION_LEVEL(table) = IMMUTABLE_RECURSION_LEVEL;
+}
+#endif
+
 #ifdef PL_DHASHMETER
 #include <math.h>
 

xpcom/glue/pldhash.h

@@ -577,6 +577,25 @@ typedef PLDHashOperator
 NS_COM_GLUE PRUint32
 PL_DHashTableEnumerate(PLDHashTable *table, PLDHashEnumerator etor, void *arg);
 
+#ifdef DEBUG
+/**
+ * Mark a table as immutable for the remainder of its lifetime.  This
+ * changes the implementation from ASSERTing one set of invariants to
+ * ASSERTing a different set.
+ *
+ * When a table is NOT marked as immutable, the table implementation
+ * asserts that the table is not mutated from its own callbacks.  It
+ * assumes the caller protects the table from being accessed on multiple
+ * threads simultaneously.
+ *
+ * When the table is marked as immutable, the re-entry assertions will
+ * no longer trigger erroneously due to multi-threaded access.  Instead,
+ * mutations will cause assertions.
+ */
+NS_COM_GLUE void
+PL_DHashMarkTableImmutable(PLDHashTable *table);
+#endif
+
 #ifdef PL_DHASHMETER
 #include <stdio.h>