wtf, heap: Safely reinitialize deleted hash table buckets

Initializing a bucket is non-atomic because the hash table is not yet
visible to the GC during initialization. When trying to recycle a
deleted bucket, the bucket is reinitialized. This introduces data races
with concurrent marking.

To resolve the data races, we should initialize atomically. For hash
table where the empty value is zero, we can atomically memset the bucket
to zero. For other cases, atomically initializing is more complicated.
Instead, in such cases, we prohibit reusing deleted bucket while marking
is in progress.

Bug: 1166985
Change-Id: I42d03371388e7cd702d2aec6c5bdfd02e2a38d4f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2643716
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846088}

third_party/blink/renderer/platform/heap/impl/heap_allocator_impl.h

@@ -66,6 +66,13 @@ class PLATFORM_EXPORT HeapAllocator {
   static void FreeHashTableBacking(void* address);
   static bool ExpandHashTableBacking(void*, size_t);
 
+  template <typename Traits>
+  static bool CanReuseHashTableDeletedBucket() {
+    if (Traits::kEmptyValueIsZero || !Traits::kCanTraceConcurrently)
+      return true;
+    return !ThreadState::Current()->IsMarkingInProgress();
+  }
+
   static bool IsAllocationAllowed() {
     return ThreadState::Current()->IsAllocationAllowed();
   }

third_party/blink/renderer/platform/wtf/allocator/partition_allocator.h

@@ -67,6 +67,11 @@ class WTF_EXPORT PartitionAllocator {
   }
 
   static inline bool ExpandHashTableBacking(void*, size_t) { return false; }
+  template <typename Traits>
+  static inline bool CanReuseHashTableDeletedBucket() {
+    return true;
+  }
+
   static void Free(void* address) { WTF::Partitions::FastFree(address); }
   template <typename T>
   static void* NewArray(size_t bytes) {

third_party/blink/renderer/platform/wtf/hash_table.h

@@ -922,6 +922,7 @@ class HashTable final
   ValueType* Reinsert(ValueType&&);
 
   static void InitializeBucket(ValueType& bucket);
+  static void ReinitializeBucket(ValueType& bucket);
   static void DeleteBucket(const ValueType& bucket) {
     bucket.~ValueType();
     Traits::ConstructDeletedValue(const_cast<ValueType&>(bucket),
@@ -1283,6 +1284,15 @@ struct HashTableBucketInitializer<false> {
     ConstructTraits<Value, Traits, Allocator>::ConstructAndNotifyElement(
         &bucket, Traits::EmptyValue());
   }
+
+  template <typename Traits, typename Allocator, typename Value>
+  static void Reinitialize(Value& bucket) {
+    // Reinitialize is used when recycling a deleted bucket. For buckets for
+    // which empty value is non-zero, this is forbidden during marking. Thus if
+    // we get here, marking is not active and we can reuse Initialize.
+    DCHECK(Allocator::template CanReuseHashTableDeletedBucket<Traits>());
+    Initialize<Traits, Allocator, Value>(bucket);
+  }
 };
 
 template <>
@@ -1296,6 +1306,19 @@ struct HashTableBucketInitializer<true> {
     // compilers.
     memset(&bucket, 0, sizeof(bucket));
   }
+
+  template <typename Traits, typename Allocator, typename Value>
+  static void Reinitialize(Value& bucket) {
+    // This initializes the bucket without copying the empty value.  That
+    // makes it possible to use this with types that don't support copying.
+    // The memset to 0 looks like a slow operation but is optimized by the
+    // compilers.
+    if (!Allocator::kIsGarbageCollected) {
+      memset(&bucket, 0, sizeof(bucket));
+      return;
+    }
+    AtomicMemzero(&bucket, sizeof(bucket));
+  }
 };
 
 template <typename Key,
@@ -1312,6 +1335,20 @@ HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::
       Traits, Allocator>(bucket);
 }
 
+template <typename Key,
+          typename Value,
+          typename Extractor,
+          typename HashFunctions,
+          typename Traits,
+          typename KeyTraits,
+          typename Allocator>
+inline void
+HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::
+    ReinitializeBucket(ValueType& bucket) {
+  HashTableBucketInitializer<Traits::kEmptyValueIsZero>::template Reinitialize<
+      Traits, Allocator>(bucket);
+}
+
 template <typename Key,
           typename Value,
           typename Extractor,
@@ -1344,6 +1381,9 @@ HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::
 
   UPDATE_ACCESS_COUNTS();
 
+  bool can_reuse_deleted_entry =
+      Allocator::template CanReuseHashTableDeletedBucket<Traits>();
+
   ValueType* deleted_entry = nullptr;
   ValueType* entry;
   while (1) {
@@ -1356,10 +1396,10 @@ HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::
       if (HashTranslator::Equal(Extractor::Extract(*entry), key))
         return AddResult(this, entry, false);
 
-      if (IsDeletedBucket(*entry))
+      if (IsDeletedBucket(*entry) && can_reuse_deleted_entry)
         deleted_entry = entry;
     } else {
-      if (IsDeletedBucket(*entry))
+      if (IsDeletedBucket(*entry) && can_reuse_deleted_entry)
         deleted_entry = entry;
       else if (HashTranslator::Equal(Extractor::Extract(*entry), key))
         return AddResult(this, entry, false);
@@ -1373,9 +1413,10 @@ HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::
   RegisterModification();
 
   if (deleted_entry) {
+    DCHECK(can_reuse_deleted_entry);
     // Overwrite any data left over from last use, using placement new or
     // memset.
-    InitializeBucket(*deleted_entry);
+    ReinitializeBucket(*deleted_entry);
     entry = deleted_entry;
     --deleted_count_;
   }
@@ -1444,7 +1485,7 @@ HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::
   RegisterModification();
 
   if (IsDeletedBucket(*entry)) {
-    InitializeBucket(*entry);
+    ReinitializeBucket(*entry);
     --deleted_count_;
   }