Clean up assertions that certain arrays can't be strong iterated.

Summary: Add a helper function that determines if an ArrayData is being used by any strong iterators, and use it in place of ad-hoc assertion code when asserting that some kinds of ArrayData should never be strong iterated. Reviewed By: jano Differential Revision: D5304324 fbshipit-source-id: 3c894038ee5b2f3759b2c0c509ca6efababcdcf6
facebook · Jun 23, 2017 · 918eda1 · 918eda1
1 parent 09f005d
commit 918eda1
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 30 deletions.
diff --git a/hphp/runtime/base/array-iterator.cpp b/hphp/runtime/base/array-iterator.cpp
@@ -729,6 +729,15 @@ void free_strong_iterators(ArrayData* ad) {
   });
 }
 
+bool has_strong_iterator(ArrayData* ad) {
+  if (LIKELY(!strong_iterators_exist())) return false;
+  bool found = false;
+  for_each_strong_iterator([&] (MIterTable::Ent& ent) {
+    if (ent.array == ad) found = true;
+  });
+  return found;
+}
+
 //////////////////////////////////////////////////////////////////////
 
 MArrayIter::MArrayIter(RefData* ref)

diff --git a/hphp/runtime/base/array-iterator.h b/hphp/runtime/base/array-iterator.h
@@ -595,6 +595,7 @@ MIterTable& miter_table();
 
 void free_strong_iterators(ArrayData*);
 void move_strong_iterators(ArrayData* dest, ArrayData* src);
+bool has_strong_iterator(ArrayData*);
 
 //////////////////////////////////////////////////////////////////////
 

diff --git a/hphp/runtime/base/mixed-array.cpp b/hphp/runtime/base/mixed-array.cpp
@@ -503,13 +503,8 @@ void MixedArray::ReleaseUncounted(ArrayData* in, size_t extra) {
       ReleaseUncountedTv(ptr->data);
     }
 
-    // We better not have strong iterators associated with uncounted
-    // arrays.
-    if (debug && UNLIKELY(strong_iterators_exist())) {
-      for_each_strong_iterator([&] (const MIterTable::Ent& miEnt) {
-        assert(miEnt.array != ad);
-      });
-    }
+    // We better not have strong iterators associated with uncounted arrays.
+    assert(!has_strong_iterator(ad));
   }
   if (UncountedMixedArrayOnHugePage()) {
     free_huge(reinterpret_cast<char*>(ad) - extra);

diff --git a/hphp/runtime/base/packed-array.cpp b/hphp/runtime/base/packed-array.cpp
@@ -523,13 +523,8 @@ void PackedArray::ReleaseUncounted(ArrayData* ad, size_t extra) {
     ReleaseUncountedTv(*ptr);
   }
 
-  // We better not have strong iterators associated with uncounted
-  // arrays.
-  if (debug && UNLIKELY(strong_iterators_exist())) {
-    for_each_strong_iterator([&] (const MIterTable::Ent& miEnt) {
-      assert(miEnt.array != ad);
-    });
-  }
+  // We better not have strong iterators associated with uncounted arrays.
+  assert(!has_strong_iterator(ad));
 
   free_huge(reinterpret_cast<char*>(ad) - extra);
 }

diff --git a/hphp/runtime/base/set-array.cpp b/hphp/runtime/base/set-array.cpp
@@ -266,14 +266,8 @@ void SetArray::Release(ArrayData* in) {
       tvDecRefGen(&elm.tv);
     }
 
-    /*
-     * We better not have strong iterators associated with keysets.
-     */
-    if (debug && UNLIKELY(strong_iterators_exist())) {
-      for_each_strong_iterator([&] (const MIterTable::Ent& miEnt) {
-        assert(miEnt.array != ad);
-      });
-    }
+    // We better not have strong iterators associated with keysets.
+    assert(!has_strong_iterator(ad));
   }
   MM().objFree(ad, ad->heapSize());
 }
@@ -298,14 +292,8 @@ void SetArray::ReleaseUncounted(ArrayData* in, size_t extra) {
       }
     }
 
-    /*
-     * We better not have strong iterators associated with keysets.
-     */
-    if (debug && UNLIKELY(strong_iterators_exist())) {
-      for_each_strong_iterator([&] (const MIterTable::Ent& miEnt) {
-        assert(miEnt.array != ad);
-      });
-    }
+    // We better not have strong iterators associated with keysets.
+    assert(!has_strong_iterator(ad));
   }
   free_huge(reinterpret_cast<char*>(ad) - extra);
 }