Merge pull request #35369 from mikeash/fix-concurrentarray-memory-order2-5.4

[5.4][Runtime] Fix incorrect memory ordering in ConcurrentReadableArray/HashMap

Author: mikeash

include/swift/Runtime/Concurrent.h

@@ -527,13 +527,18 @@ template <class ElemTy> struct ConcurrentReadableArray {
 
       storage = newStorage;
       Capacity = newCapacity;
-      Elements.store(storage, std::memory_order_release);
+
+      // Use seq_cst here to ensure that the subsequent load of ReaderCount is
+      // ordered after this store. If ReaderCount is loaded first, then a new
+      // reader could come in between that load and this store, and then we
+      // could end up freeing the old storage pointer while it's still in use.
+      Elements.store(storage, std::memory_order_seq_cst);
     }
 
     new(&storage->data()[count]) ElemTy(elem);
     storage->Count.store(count + 1, std::memory_order_release);
 
-    if (ReaderCount.load(std::memory_order_acquire) == 0)
+    if (ReaderCount.load(std::memory_order_seq_cst) == 0)
       deallocateFreeList();
   }
 
@@ -848,7 +853,7 @@ struct ConcurrentReadableHashMap {
   /// Free all the arrays in the free lists if there are no active readers. If
   /// there are active readers, do nothing.
   void deallocateFreeListIfSafe() {
-    if (ReaderCount.load(std::memory_order_acquire) == 0)
+    if (ReaderCount.load(std::memory_order_seq_cst) == 0)
       FreeListNode::freeAll(&FreeList);
   }
 
@@ -866,7 +871,11 @@ struct ConcurrentReadableHashMap {
       FreeListNode::add(&FreeList, elements);
     }
 
-    Elements.store(newElements, std::memory_order_release);
+    // Use seq_cst here to ensure that the subsequent load of ReaderCount is
+    // ordered after this store. If ReaderCount is loaded first, then a new
+    // reader could come in between that load and this store, and then we
+    // could end up freeing the old elements pointer while it's still in use.
+    Elements.store(newElements, std::memory_order_seq_cst);
     return newElements;
   }
 
@@ -900,7 +909,11 @@ struct ConcurrentReadableHashMap {
       newIndices.storeIndexAt(nullptr, index, newI, std::memory_order_relaxed);
     }
 
-    Indices.store(newIndices.Value, std::memory_order_release);
+    // Use seq_cst here to ensure that the subsequent load of ReaderCount is
+    // ordered after this store. If ReaderCount is loaded first, then a new
+    // reader could come in between that load and this store, and then we
+    // could end up freeing the old indices pointer while it's still in use.
+    Indices.store(newIndices.Value, std::memory_order_seq_cst);
 
     if (auto *ptr = indices.pointer())
       FreeListNode::add(&FreeList, ptr);