commit bb816d66aabb9c2a2e095517d2013041116332db
author Mathieu Chartier <mathieuc@google.com> Wed Sep 07 10:17:46 2016 -0700
committer Mathieu Chartier <mathieuc@google.com> Thu Sep 08 11:17:17 2016 -0700
tree 1d51444ec1d6febfba8c2d6e9502bf76939261fa
parent ddac139d83bfb121d1536fb40ab48c9bee6c25b1

Add transactions for string resolve

Fixes a bug where resolved strings can be left in the dex cache after
a transaction is rolled back even though the interned string was
removed.

Added test in transaction_test.

Bug: 31239436

Test: test-art-host

Change-Id: I42c67bcefeae8db134cde34c480261f52db4102e

runtime/transaction.cc

diff --git a/runtime/transaction.cc b/runtime/transaction.cc
index d91860b..9f8d981 100644
--- a/runtime/transaction.cc
+++ b/runtime/transaction.cc
@@ -49,13 +49,15 @@
     for (auto it : array_logs_) {
       array_values_count += it.second.Size();
     }
-    size_t string_count = intern_string_logs_.size();
+    size_t intern_string_count = intern_string_logs_.size();
+    size_t resolve_string_count = resolve_string_logs_.size();
     LOG(INFO) << "Transaction::~Transaction"
               << ": objects_count=" << objects_count
               << ", field_values_count=" << field_values_count
               << ", array_count=" << array_count
               << ", array_values_count=" << array_values_count
-              << ", string_count=" << string_count;
+              << ", intern_string_count=" << intern_string_count
+              << ", resolve_string_count=" << resolve_string_count;
   }
 }
 
@@ -165,6 +167,13 @@
   array_log.LogValue(index, value);
 }
 
+void Transaction::RecordResolveString(mirror::DexCache* dex_cache, uint32_t string_idx) {
+  DCHECK(dex_cache != nullptr);
+  DCHECK_LT(string_idx, dex_cache->GetDexFile()->NumStringIds());
+  MutexLock mu(Thread::Current(), log_lock_);
+  resolve_string_logs_.push_back(ResolveStringLog(dex_cache, string_idx));
+}
+
 void Transaction::RecordStrongStringInsertion(mirror::String* s) {
   InternStringLog log(s, InternStringLog::kStrongString, InternStringLog::kInsert);
   LogInternedString(log);
@@ -200,6 +209,7 @@
   UndoObjectModifications();
   UndoArrayModifications();
   UndoInternStringTableModifications();
+  UndoResolveStringModifications();
 }
 
 void Transaction::UndoObjectModifications() {
@@ -230,11 +240,19 @@
   intern_string_logs_.clear();
 }
 
+void Transaction::UndoResolveStringModifications() {
+  for (ResolveStringLog& string_log : resolve_string_logs_) {
+    string_log.Undo();
+  }
+  resolve_string_logs_.clear();
+}
+
 void Transaction::VisitRoots(RootVisitor* visitor) {
   MutexLock mu(Thread::Current(), log_lock_);
   VisitObjectLogs(visitor);
   VisitArrayLogs(visitor);
-  VisitStringLogs(visitor);
+  VisitInternStringLogs(visitor);
+  VisitResolveStringLogs(visitor);
 }
 
 void Transaction::VisitObjectLogs(RootVisitor* visitor) {
@@ -292,12 +310,18 @@
   }
 }
 
-void Transaction::VisitStringLogs(RootVisitor* visitor) {
+void Transaction::VisitInternStringLogs(RootVisitor* visitor) {
   for (InternStringLog& log : intern_string_logs_) {
     log.VisitRoots(visitor);
   }
 }
 
+void Transaction::VisitResolveStringLogs(RootVisitor* visitor) {
+  for (ResolveStringLog& log : resolve_string_logs_) {
+    log.VisitRoots(visitor);
+  }
+}
+
 void Transaction::ObjectLog::LogBooleanValue(MemberOffset offset, uint8_t value, bool is_volatile) {
   LogValue(ObjectLog::kBoolean, offset, value, is_volatile);
 }
@@ -481,6 +505,21 @@
   visitor->VisitRoot(reinterpret_cast<mirror::Object**>(&str_), RootInfo(kRootInternedString));
 }
 
+void Transaction::ResolveStringLog::Undo() {
+  dex_cache_.Read()->ClearString(string_idx_);
+}
+
+Transaction::ResolveStringLog::ResolveStringLog(mirror::DexCache* dex_cache, uint32_t string_idx)
+    : dex_cache_(dex_cache),
+      string_idx_(string_idx) {
+  DCHECK(dex_cache != nullptr);
+  DCHECK_LT(string_idx_, dex_cache->GetDexFile()->NumStringIds());
+}
+
+void Transaction::ResolveStringLog::VisitRoots(RootVisitor* visitor) {
+  dex_cache_.VisitRoot(visitor, RootInfo(kRootVMInternal));
+}
+
 void Transaction::ArrayLog::LogValue(size_t index, uint64_t value) {
   auto it = array_values_.find(index);
   if (it == array_values_.end()) {