diff options
| -rw-r--r-- | runtime/class_linker.cc | 8 | ||||
| -rw-r--r-- | runtime/intern_table-inl.h | 12 | ||||
| -rw-r--r-- | runtime/intern_table.h | 8 |
3 files changed, 16 insertions, 12 deletions
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index f43791ab06..e3dfdb3256 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -1486,7 +1486,6 @@ void AppImageLoadingHelper::HandleAppImageStrings(gc::space::ImageSpace* space) // the strings they point to. ScopedTrace timing("AppImage:InternString"); - Thread* const self = Thread::Current(); InternTable* const intern_table = Runtime::Current()->GetInternTable(); // Add the intern table, removing any conflicts. For conflicts, store the new address in a map @@ -1494,13 +1493,14 @@ void AppImageLoadingHelper::HandleAppImageStrings(gc::space::ImageSpace* space) // TODO: Optimize with a bitmap or bloom filter SafeMap<mirror::String*, mirror::String*> intern_remap; intern_table->AddImageStringsToTable(space, [&](InternTable::UnorderedSet& interns) - REQUIRES_SHARED(Locks::mutator_lock_) { + REQUIRES_SHARED(Locks::mutator_lock_) + REQUIRES(Locks::intern_table_lock_) { VLOG(image) << "AppImage:stringsInInternTableSize = " << interns.size(); for (auto it = interns.begin(); it != interns.end(); ) { ObjPtr<mirror::String> string = it->Read(); - ObjPtr<mirror::String> existing = intern_table->LookupWeak(self, string); + ObjPtr<mirror::String> existing = intern_table->LookupWeakLocked(string); if (existing == nullptr) { - existing = intern_table->LookupStrong(self, string); + existing = intern_table->LookupStrongLocked(string); } if (existing != nullptr) { intern_remap.Put(string.Ptr(), existing.Ptr()); diff --git a/runtime/intern_table-inl.h b/runtime/intern_table-inl.h index 8c7fb42952..84754fafb8 100644 --- a/runtime/intern_table-inl.h +++ b/runtime/intern_table-inl.h @@ -39,11 +39,15 @@ template <typename Visitor> inline size_t InternTable::AddTableFromMemory(const uint8_t* ptr, const Visitor& visitor) { size_t read_count = 0; UnorderedSet set(ptr, /*make copy*/false, &read_count); - // Visit the unordered set, may remove elements. - visitor(set); - if (!set.empty()) { + { + // Hold the lock while calling the visitor to prevent possible race + // conditions with another thread adding intern strings. MutexLock mu(Thread::Current(), *Locks::intern_table_lock_); - strong_interns_.AddInternStrings(std::move(set)); + // Visit the unordered set, may remove elements. + visitor(set); + if (!set.empty()) { + strong_interns_.AddInternStrings(std::move(set)); + } } return read_count; } diff --git a/runtime/intern_table.h b/runtime/intern_table.h index 1bc89a1048..e918a458b9 100644 --- a/runtime/intern_table.h +++ b/runtime/intern_table.h @@ -145,11 +145,15 @@ class InternTable { ObjPtr<mirror::String> LookupStrong(Thread* self, uint32_t utf16_length, const char* utf8_data) REQUIRES(!Locks::intern_table_lock_) REQUIRES_SHARED(Locks::mutator_lock_); + ObjPtr<mirror::String> LookupStrongLocked(ObjPtr<mirror::String> s) + REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::intern_table_lock_); // Lookup a weak intern, returns null if not found. ObjPtr<mirror::String> LookupWeak(Thread* self, ObjPtr<mirror::String> s) REQUIRES(!Locks::intern_table_lock_) REQUIRES_SHARED(Locks::mutator_lock_); + ObjPtr<mirror::String> LookupWeakLocked(ObjPtr<mirror::String> s) + REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::intern_table_lock_); // Total number of interned strings. size_t Size() const REQUIRES(!Locks::intern_table_lock_); @@ -250,10 +254,6 @@ class InternTable { size_t AddTableFromMemory(const uint8_t* ptr, const Visitor& visitor) REQUIRES(!Locks::intern_table_lock_) REQUIRES_SHARED(Locks::mutator_lock_); - ObjPtr<mirror::String> LookupStrongLocked(ObjPtr<mirror::String> s) - REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::intern_table_lock_); - ObjPtr<mirror::String> LookupWeakLocked(ObjPtr<mirror::String> s) - REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::intern_table_lock_); ObjPtr<mirror::String> InsertStrong(ObjPtr<mirror::String> s) REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::intern_table_lock_); ObjPtr<mirror::String> InsertWeak(ObjPtr<mirror::String> s) |