Fix thread race when fetching the ProfilingInfo object.

Problem is:
1) Compiler fetches the ProfilingInfo of A, it's null.
2) Mutator creates the ProfilingInfo.
3) Compiler notifies it's not using A anymore, calls
   ProfilingInfo::DecrementInlineUse -> Crash as we expected
   ProfilingInfo::IncrementUse to be called before.

Also update some namings to better reflect what is going on.

Change-Id: I55ea4c5d81988131467095e18a0d13a8be9d0ef7

diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index bbdac26..d861e39 100644 (file)
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -224,16 +224,29 @@ static uint32_t FindClassIndexIn(mirror::Class* cls,
 
 class ScopedProfilingInfoInlineUse {
  public:
-  explicit ScopedProfilingInfoInlineUse(ArtMethod* method) : method_(method) {
-    Runtime::Current()->GetJit()->GetCodeCache()->NotifyInliningOf(method_, Thread::Current());
+  explicit ScopedProfilingInfoInlineUse(ArtMethod* method, Thread* self)
+      : method_(method),
+        self_(self),
+        // Fetch the profiling info ahead of using it. If it's null when fetching,
+        // we should not call JitCodeCache::DoneInlining.
+        profiling_info_(
+            Runtime::Current()->GetJit()->GetCodeCache()->NotifyCompilerUse(method, self)) {
   }
 
   ~ScopedProfilingInfoInlineUse() {
-    Runtime::Current()->GetJit()->GetCodeCache()->DoneInlining(method_, Thread::Current());
+    if (profiling_info_ != nullptr) {
+      size_t pointer_size = Runtime::Current()->GetClassLinker()->GetImagePointerSize();
+      DCHECK_EQ(profiling_info_, method_->GetProfilingInfo(pointer_size));
+      Runtime::Current()->GetJit()->GetCodeCache()->DoneCompilerUse(method_, self_);
+    }
   }
 
+  ProfilingInfo* GetProfilingInfo() const { return profiling_info_; }
+
  private:
   ArtMethod* const method_;
+  Thread* const self_;
+  ProfilingInfo* const profiling_info_;
 };
 
 bool HInliner::TryInline(HInvoke* invoke_instruction) {
@@ -287,15 +300,14 @@ bool HInliner::TryInline(HInvoke* invoke_instruction) {
 
   // Check if we can use an inline cache.
   ArtMethod* caller = graph_->GetArtMethod();
-  size_t pointer_size = class_linker->GetImagePointerSize();
   if (Runtime::Current()->UseJit()) {
     // Under JIT, we should always know the caller.
     DCHECK(caller != nullptr);
-    ScopedProfilingInfoInlineUse spiis(caller);
-    ProfilingInfo* profiling_info = caller->GetProfilingInfo(pointer_size);
+    ScopedProfilingInfoInlineUse spiis(caller, soa.Self());
+    ProfilingInfo* profiling_info = spiis.GetProfilingInfo();
     if (profiling_info != nullptr) {
       const InlineCache& ic = *profiling_info->GetInlineCache(invoke_instruction->GetDexPc());
-      if (ic.IsUnitialized()) {
+      if (ic.IsUninitialized()) {
         VLOG(compiler) << "Interface or virtual call to "
                        << PrettyMethod(method_index, caller_dex_file)
                        << " is not hit and not inlined";

diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
index 050bb68..af47da6 100644 (file)
--- a/runtime/jit/jit_code_cache.cc
+++ b/runtime/jit/jit_code_cache.cc
@@ -928,20 +928,20 @@ bool JitCodeCache::NotifyCompilationOf(ArtMethod* method, Thread* self, bool osr
   return true;
 }
 
-void JitCodeCache::NotifyInliningOf(ArtMethod* method, Thread* self) {
+ProfilingInfo* JitCodeCache::NotifyCompilerUse(ArtMethod* method, Thread* self) {
   MutexLock mu(self, lock_);
   ProfilingInfo* info = method->GetProfilingInfo(sizeof(void*));
   if (info != nullptr) {
     info->IncrementInlineUse();
   }
+  return info;
 }
 
-void JitCodeCache::DoneInlining(ArtMethod* method, Thread* self) {
+void JitCodeCache::DoneCompilerUse(ArtMethod* method, Thread* self) {
   MutexLock mu(self, lock_);
   ProfilingInfo* info = method->GetProfilingInfo(sizeof(void*));
-  if (info != nullptr) {
-    info->DecrementInlineUse();
-  }
+  DCHECK(info != nullptr);
+  info->DecrementInlineUse();
 }
 
 void JitCodeCache::DoneCompiling(ArtMethod* method, Thread* self ATTRIBUTE_UNUSED) {

diff --git a/runtime/jit/jit_code_cache.h b/runtime/jit/jit_code_cache.h
index 113bebf..98dd70d 100644 (file)
--- a/runtime/jit/jit_code_cache.h
+++ b/runtime/jit/jit_code_cache.h
@@ -71,7 +71,11 @@ class JitCodeCache {
       SHARED_REQUIRES(Locks::mutator_lock_)
       REQUIRES(!lock_);
 
-  void NotifyInliningOf(ArtMethod* method, Thread* self)
+  // Notify to the code cache that the compiler wants to use the
+  // profiling info of `method` to drive optimizations,
+  // and therefore ensure the returned profiling info object is not
+  // collected.
+  ProfilingInfo* NotifyCompilerUse(ArtMethod* method, Thread* self)
       SHARED_REQUIRES(Locks::mutator_lock_)
       REQUIRES(!lock_);
 
@@ -79,7 +83,7 @@ class JitCodeCache {
       SHARED_REQUIRES(Locks::mutator_lock_)
       REQUIRES(!lock_);
 
-  void DoneInlining(ArtMethod* method, Thread* self)
+  void DoneCompilerUse(ArtMethod* method, Thread* self)
       SHARED_REQUIRES(Locks::mutator_lock_)
       REQUIRES(!lock_);
 

diff --git a/runtime/jit/profiling_info.h b/runtime/jit/profiling_info.h
index 73c1a1e..55d627a 100644 (file)
--- a/runtime/jit/profiling_info.h
+++ b/runtime/jit/profiling_info.h
@@ -56,10 +56,11 @@ class InlineCache {
   mirror::Class* GetMonomorphicType() const SHARED_REQUIRES(Locks::mutator_lock_) {
     // Note that we cannot ensure the inline cache is actually monomorphic
     // at this point, as other threads may have updated it.
+    DCHECK(!classes_[0].IsNull());
     return classes_[0].Read();
   }
 
-  bool IsUnitialized() const {
+  bool IsUninitialized() const {
     return classes_[0].IsNull();
   }