Use atomics for find_array_class_cache_

find_array_class_cache_ is used to cache the array classes. This is
accessed concurrently by mutator threads that are looking for an array
class. These threads both read entries and update the cache with the new
array class if it wasn't found in the cache. GC also accesses this cache
to clear it when visiting the roots of class linker. GC uses std::fill_n
to clear the cache. Under ideal circumstances these operations could
work as expected and the updates to each entry could be atomic. Though
some valid compiler optimizations could break this and it is possible we
see a partially initialized / cleared entry. To prevent these we use
atomics to make it safe to access concurrently.

Bug: 330843930
Test: art/test.py
Change-Id: I493e0b0d47bf03603a4970855608cbe82a87c1a4

3 files changed, 11 insertions, 5 deletions

diff --git a/runtime/class_linker-inl.h b/runtime/class_linker-inl.h
index 6951e35791..6461f54f5f 100644
--- a/runtime/class_linker-inl.h
+++ b/runtime/class_linker-inl.h
@@ -43,7 +43,8 @@ inline ObjPtr<mirror::Class> ClassLinker::FindArrayClass(Thread* self,
                                                          ObjPtr<mirror::Class> element_class) {
   for (size_t i = 0; i < kFindArrayCacheSize; ++i) {
     // Read the cached array class once to avoid races with other threads setting it.
-    ObjPtr<mirror::Class> array_class = find_array_class_cache_[i].Read();
+    ObjPtr<mirror::Class> array_class =
+        find_array_class_cache_[i].load(std::memory_order_acquire).Read();
     if (array_class != nullptr && array_class->GetComponentType() == element_class) {
       return array_class;
     }
@@ -57,7 +58,8 @@ inline ObjPtr<mirror::Class> ClassLinker::FindArrayClass(Thread* self,
   if (array_class != nullptr) {
     // Benign races in storing array class and incrementing index.
     size_t victim_index = find_array_class_cache_next_victim_;
-    find_array_class_cache_[victim_index] = GcRoot<mirror::Class>(array_class);
+    find_array_class_cache_[victim_index].store(GcRoot<mirror::Class>(array_class),
+                                                std::memory_order_release);
     find_array_class_cache_next_victim_ = (victim_index + 1) % kFindArrayCacheSize;
   } else {
     // We should have a NoClassDefFoundError.
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 829b3e3a4f..585e2b3818 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -643,7 +643,9 @@ ClassLinker::ClassLinker(InternTable* intern_table, bool fast_class_not_found_ex
   CHECK(intern_table_ != nullptr);
   static_assert(kFindArrayCacheSize == arraysize(find_array_class_cache_),
                 "Array cache size wrong.");
-  std::fill_n(find_array_class_cache_, kFindArrayCacheSize, GcRoot<mirror::Class>(nullptr));
+  for (size_t i = 0; i < kFindArrayCacheSize; i++) {
+    find_array_class_cache_[i].store(GcRoot<mirror::Class>(nullptr), std::memory_order_relaxed);
+  }
 }
 
 void ClassLinker::CheckSystemClass(Thread* self, Handle<mirror::Class> c1, const char* descriptor) {
@@ -10927,7 +10929,9 @@ jobject ClassLinker::CreatePathClassLoader(Thread* self,
 }
 
 void ClassLinker::DropFindArrayClassCache() {
-  std::fill_n(find_array_class_cache_, kFindArrayCacheSize, GcRoot<mirror::Class>(nullptr));
+  for (size_t i = 0; i < kFindArrayCacheSize; i++) {
+    find_array_class_cache_[i].store(GcRoot<mirror::Class>(nullptr), std::memory_order_relaxed);
+  }
   find_array_class_cache_next_victim_ = 0;
 }
 
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 8b781b90f9..0fe1aaf7a3 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -1466,7 +1466,7 @@ class ClassLinker {
   // A cache of the last FindArrayClass results. The cache serves to avoid creating array class
   // descriptors for the sake of performing FindClass.
   static constexpr size_t kFindArrayCacheSize = 16;
-  GcRoot<mirror::Class> find_array_class_cache_[kFindArrayCacheSize];
+  std::atomic<GcRoot<mirror::Class>> find_array_class_cache_[kFindArrayCacheSize];
   size_t find_array_class_cache_next_victim_;
 
   bool init_done_;