Merge "Cleanup thread access in StackVisitor"

6 files changed, 31 insertions, 21 deletions

diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index d5691af800..ad69676ea6 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -2443,6 +2443,8 @@ class FindFrameVisitor FINAL : public StackVisitor {
  private:
   const JDWP::FrameId frame_id_;
   JDWP::JdwpError error_;
+
+  DISALLOW_COPY_AND_ASSIGN(FindFrameVisitor);
 };
 
 JDWP::JdwpError Dbg::GetLocalValues(JDWP::Request* request, JDWP::ExpandBuf* pReply) {
@@ -2822,7 +2824,6 @@ class CatchLocationFinder : public StackVisitor {
   CatchLocationFinder(Thread* self, const Handle<mirror::Throwable>& exception, Context* context)
       SHARED_REQUIRES(Locks::mutator_lock_)
     : StackVisitor(self, context, StackVisitor::StackWalkKind::kIncludeInlinedFrames),
-      self_(self),
       exception_(exception),
       handle_scope_(self),
       this_at_throw_(handle_scope_.NewHandle<mirror::Object>(nullptr)),
@@ -2852,7 +2853,7 @@ class CatchLocationFinder : public StackVisitor {
     }
 
     if (dex_pc != DexFile::kDexNoIndex) {
-      StackHandleScope<1> hs(self_);
+      StackHandleScope<1> hs(GetThread());
       uint32_t found_dex_pc;
       Handle<mirror::Class> exception_class(hs.NewHandle(exception_->GetClass()));
       bool unused_clear_exception;
@@ -2887,7 +2888,6 @@ class CatchLocationFinder : public StackVisitor {
   }
 
  private:
-  Thread* const self_;
   const Handle<mirror::Throwable>& exception_;
   StackHandleScope<1> handle_scope_;
   MutableHandle<mirror::Object> this_at_throw_;
diff --git a/runtime/profiler.cc b/runtime/profiler.cc
index 7e8c551ab2..6a77a9ed83 100644
--- a/runtime/profiler.cc
+++ b/runtime/profiler.cc
@@ -57,7 +57,8 @@ volatile bool BackgroundMethodSamplingProfiler::shutting_down_ = false;
 class BoundedStackVisitor : public StackVisitor {
  public:
   BoundedStackVisitor(std::vector<std::pair<ArtMethod*, uint32_t>>* stack,
-      Thread* thread, uint32_t max_depth)
+                      Thread* thread,
+                      uint32_t max_depth)
       SHARED_REQUIRES(Locks::mutator_lock_)
       : StackVisitor(thread, nullptr, StackVisitor::StackWalkKind::kIncludeInlinedFrames),
         stack_(stack),
@@ -80,9 +81,11 @@ class BoundedStackVisitor : public StackVisitor {
   }
 
  private:
-  std::vector<std::pair<ArtMethod*, uint32_t>>* stack_;
+  std::vector<std::pair<ArtMethod*, uint32_t>>* const stack_;
   const uint32_t max_depth_;
   uint32_t depth_;
+
+  DISALLOW_COPY_AND_ASSIGN(BoundedStackVisitor);
 };
 
 // This is called from either a thread list traversal or from a checkpoint.  Regardless
diff --git a/runtime/quick_exception_handler.cc b/runtime/quick_exception_handler.cc
index c905b63c6c..d797d2ad60 100644
--- a/runtime/quick_exception_handler.cc
+++ b/runtime/quick_exception_handler.cc
@@ -47,7 +47,6 @@ class CatchBlockStackVisitor FINAL : public StackVisitor {
                          QuickExceptionHandler* exception_handler)
       SHARED_REQUIRES(Locks::mutator_lock_)
       : StackVisitor(self, context, StackVisitor::StackWalkKind::kIncludeInlinedFrames),
-        self_(self),
         exception_(exception),
         exception_handler_(exception_handler) {
   }
@@ -90,7 +89,7 @@ class CatchBlockStackVisitor FINAL : public StackVisitor {
     }
     if (dex_pc != DexFile::kDexNoIndex) {
       bool clear_exception = false;
-      StackHandleScope<1> hs(self_);
+      StackHandleScope<1> hs(GetThread());
       Handle<mirror::Class> to_find(hs.NewHandle((*exception_)->GetClass()));
       uint32_t found_dex_pc = method->FindCatchBlock(to_find, dex_pc, &clear_exception);
       exception_handler_->SetClearException(clear_exception);
@@ -106,7 +105,6 @@ class CatchBlockStackVisitor FINAL : public StackVisitor {
     return true;  // Continue stack walk.
   }
 
-  Thread* const self_;
   // The exception we're looking for the catch block of.
   Handle<mirror::Throwable>* exception_;
   // The quick exception handler we're visiting for.
@@ -256,7 +254,6 @@ class DeoptimizeStackVisitor FINAL : public StackVisitor {
   DeoptimizeStackVisitor(Thread* self, Context* context, QuickExceptionHandler* exception_handler)
       SHARED_REQUIRES(Locks::mutator_lock_)
       : StackVisitor(self, context, StackVisitor::StackWalkKind::kIncludeInlinedFrames),
-        self_(self),
         exception_handler_(exception_handler),
         prev_shadow_frame_(nullptr),
         stacked_shadow_frame_pushed_(false) {
@@ -273,7 +270,8 @@ class DeoptimizeStackVisitor FINAL : public StackVisitor {
         // In case there is no deoptimized shadow frame for this upcall, we still
         // need to push a nullptr to the stack since there is always a matching pop after
         // the long jump.
-        self_->PushStackedShadowFrame(nullptr, StackedShadowFrameType::kDeoptimizationShadowFrame);
+        GetThread()->PushStackedShadowFrame(nullptr,
+                                            StackedShadowFrameType::kDeoptimizationShadowFrame);
         stacked_shadow_frame_pushed_ = true;
       }
       return false;  // End stack walk.
@@ -302,18 +300,19 @@ class DeoptimizeStackVisitor FINAL : public StackVisitor {
     CHECK(code_item != nullptr) << "No code item for " << PrettyMethod(m);
     uint16_t num_regs = code_item->registers_size_;
     uint32_t dex_pc = GetDexPc();
-    StackHandleScope<2> hs(self_);  // Dex cache, class loader and method.
+    StackHandleScope<2> hs(GetThread());  // Dex cache, class loader and method.
     mirror::Class* declaring_class = m->GetDeclaringClass();
     Handle<mirror::DexCache> h_dex_cache(hs.NewHandle(declaring_class->GetDexCache()));
     Handle<mirror::ClassLoader> h_class_loader(hs.NewHandle(declaring_class->GetClassLoader()));
-    verifier::MethodVerifier verifier(self_, h_dex_cache->GetDexFile(), h_dex_cache, h_class_loader,
-                                      &m->GetClassDef(), code_item, m->GetDexMethodIndex(),
-                                      m, m->GetAccessFlags(), true, true, true, true);
+    verifier::MethodVerifier verifier(GetThread(), h_dex_cache->GetDexFile(), h_dex_cache,
+                                      h_class_loader, &m->GetClassDef(), code_item,
+                                      m->GetDexMethodIndex(), m, m->GetAccessFlags(), true, true,
+                                      true, true);
     bool verifier_success = verifier.Verify();
     CHECK(verifier_success) << PrettyMethod(m);
     ShadowFrame* new_frame = ShadowFrame::CreateDeoptimizedFrame(num_regs, nullptr, m, dex_pc);
     {
-      ScopedStackedShadowFramePusher pusher(self_, new_frame,
+      ScopedStackedShadowFramePusher pusher(GetThread(), new_frame,
                                             StackedShadowFrameType::kShadowFrameUnderConstruction);
       const std::vector<int32_t> kinds(verifier.DescribeVRegs(dex_pc));
 
@@ -420,13 +419,13 @@ class DeoptimizeStackVisitor FINAL : public StackVisitor {
       // Will be popped after the long jump after DeoptimizeStack(),
       // right before interpreter::EnterInterpreterFromDeoptimize().
       stacked_shadow_frame_pushed_ = true;
-      self_->PushStackedShadowFrame(new_frame, StackedShadowFrameType::kDeoptimizationShadowFrame);
+      GetThread()->PushStackedShadowFrame(new_frame,
+                                          StackedShadowFrameType::kDeoptimizationShadowFrame);
     }
     prev_shadow_frame_ = new_frame;
     return true;
   }
 
-  Thread* const self_;
   QuickExceptionHandler* const exception_handler_;
   ShadowFrame* prev_shadow_frame_;
   bool stacked_shadow_frame_pushed_;
diff --git a/runtime/stack.h b/runtime/stack.h
index 25627383fe..5bbf003e5d 100644
--- a/runtime/stack.h
+++ b/runtime/stack.h
@@ -441,6 +441,10 @@ class StackVisitor {
   void WalkStack(bool include_transitions = false)
       SHARED_REQUIRES(Locks::mutator_lock_);
 
+  Thread* GetThread() const {
+    return thread_;
+  }
+
   ArtMethod* GetMethod() const SHARED_REQUIRES(Locks::mutator_lock_);
 
   bool IsShadowFrame() const {
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 86ac1407da..6e10368c84 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -1185,7 +1185,6 @@ struct StackDumpVisitor : public StackVisitor {
       SHARED_REQUIRES(Locks::mutator_lock_)
       : StackVisitor(thread_in, context, StackVisitor::StackWalkKind::kIncludeInlinedFrames),
         os(os_in),
-        thread(thread_in),
         can_allocate(can_allocate_in),
         last_method(nullptr),
         last_line_number(0),
@@ -1233,7 +1232,7 @@ struct StackDumpVisitor : public StackVisitor {
       }
       os << "\n";
       if (frame_count == 0) {
-        Monitor::DescribeWait(os, thread);
+        Monitor::DescribeWait(os, GetThread());
       }
       if (can_allocate) {
         // Visit locks, but do not abort on errors. This would trigger a nested abort.
@@ -1269,7 +1268,6 @@ struct StackDumpVisitor : public StackVisitor {
   }
 
   std::ostream& os;
-  const Thread* thread;
   const bool can_allocate;
   ArtMethod* last_method;
   int last_line_number;
@@ -1825,6 +1823,8 @@ class CountStackDepthVisitor : public StackVisitor {
   uint32_t depth_;
   uint32_t skip_depth_;
   bool skipping_;
+
+  DISALLOW_COPY_AND_ASSIGN(CountStackDepthVisitor);
 };
 
 template<bool kTransactionActive>
@@ -1891,7 +1891,9 @@ class BuildInternalStackTraceVisitor : public StackVisitor {
   // An array of the methods on the stack, the last entries are the dex PCs.
   mirror::PointerArray* trace_;
   // For cross compilation.
-  size_t pointer_size_;
+  const size_t pointer_size_;
+
+  DISALLOW_COPY_AND_ASSIGN(BuildInternalStackTraceVisitor);
 };
 
 template<bool kTransactionActive>
diff --git a/runtime/trace.cc b/runtime/trace.cc
index 5a44947d35..e2743ceb13 100644
--- a/runtime/trace.cc
+++ b/runtime/trace.cc
@@ -73,6 +73,8 @@ class BuildStackTraceVisitor : public StackVisitor {
 
  private:
   std::vector<ArtMethod*>* const method_trace_;
+
+  DISALLOW_COPY_AND_ASSIGN(BuildStackTraceVisitor);
 };
 
 static const char     kTraceTokenChar             = '*';