Extensions to check JNI.

Ensure critical lock isn't held when returning from a down-call.
Log a warning if the critical lock is held for a significant period of
time.
Refactor JNIEnvExt to be a class rather than a struct.

Test: mma test-art-host

Change-Id: I4d149cb04d3a7308a22b92b196e51e2f1ae17ede

29 files changed, 268 insertions, 165 deletions

diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index b3d956137a..f5d09dea45 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -1695,10 +1695,10 @@ class Dex2Oat FINAL {
       CHECK(class_loader != nullptr);
       ScopedObjectAccess soa(Thread::Current());
       // Unload class loader to free RAM.
-      jweak weak_class_loader = soa.Env()->vm->AddWeakGlobalRef(
+      jweak weak_class_loader = soa.Env()->GetVm()->AddWeakGlobalRef(
           soa.Self(),
           soa.Decode<mirror::ClassLoader>(class_loader));
-      soa.Env()->vm->DeleteGlobalRef(soa.Self(), class_loader);
+      soa.Env()->GetVm()->DeleteGlobalRef(soa.Self(), class_loader);
       runtime_->GetHeap()->CollectGarbage(/*clear_soft_references*/ true);
       ObjPtr<mirror::ClassLoader> decoded_weak = soa.Decode<mirror::ClassLoader>(weak_class_loader);
       if (decoded_weak != nullptr) {
@@ -2898,7 +2898,7 @@ class ScopedGlobalRef {
   ~ScopedGlobalRef() {
     if (obj_ != nullptr) {
       ScopedObjectAccess soa(Thread::Current());
-      soa.Env()->vm->DeleteGlobalRef(soa.Self(), obj_);
+      soa.Env()->GetVm()->DeleteGlobalRef(soa.Self(), obj_);
     }
   }
 
diff --git a/openjdkjvm/OpenjdkJvm.cc b/openjdkjvm/OpenjdkJvm.cc
index 1b8233aae8..ff839f5126 100644
--- a/openjdkjvm/OpenjdkJvm.cc
+++ b/openjdkjvm/OpenjdkJvm.cc
@@ -387,7 +387,7 @@ JNIEXPORT void JVM_Interrupt(JNIEnv* env, jobject jthread) {
 
 JNIEXPORT jboolean JVM_IsInterrupted(JNIEnv* env, jobject jthread, jboolean clearInterrupted) {
   if (clearInterrupted) {
-    return static_cast<art::JNIEnvExt*>(env)->self->Interrupted() ? JNI_TRUE : JNI_FALSE;
+    return static_cast<art::JNIEnvExt*>(env)->GetSelf()->Interrupted() ? JNI_TRUE : JNI_FALSE;
   } else {
     art::ScopedFastNativeObjectAccess soa(env);
     art::MutexLock mu(soa.Self(), *art::Locks::thread_list_lock_);
diff --git a/openjdkjvmti/ti_class.cc b/openjdkjvmti/ti_class.cc
index e69c78bab1..60ab0a50e6 100644
--- a/openjdkjvmti/ti_class.cc
+++ b/openjdkjvmti/ti_class.cc
@@ -490,7 +490,7 @@ struct ClassCallback : public art::ClassLoadCallback {
 
         // Fix up the local table with a root visitor.
         RootUpdater local_update(local->input_, local->output_);
-        t->GetJniEnv()->locals.VisitRoots(
+        t->GetJniEnv()->VisitJniLocalRoots(
             &local_update, art::RootInfo(art::kRootJNILocal, t->GetThreadId()));
       }
 
diff --git a/openjdkjvmti/ti_stack.cc b/openjdkjvmti/ti_stack.cc
index b43eaa0286..8ee150ee3e 100644
--- a/openjdkjvmti/ti_stack.cc
+++ b/openjdkjvmti/ti_stack.cc
@@ -891,7 +891,7 @@ struct MonitorInfoClosure : public art::Closure {
     visitor.WalkStack(/* include_transitions */ false);
     // Find any other monitors, including ones acquired in native code.
     art::RootInfo root_info(art::kRootVMInternal);
-    target->GetJniEnv()->monitors.VisitRoots(&visitor, root_info);
+    target->GetJniEnv()->VisitMonitorRoots(&visitor, root_info);
     err_ = handle_results_(soa_, visitor);
   }
 
diff --git a/runtime/base/mutex.cc b/runtime/base/mutex.cc
index 7324dff974..e88ed68ef1 100644
--- a/runtime/base/mutex.cc
+++ b/runtime/base/mutex.cc
@@ -959,7 +959,7 @@ void ConditionVariable::WaitHoldingLocks(Thread* self) {
   }
   if (self != nullptr) {
     JNIEnvExt* const env = self->GetJniEnv();
-    if (UNLIKELY(env != nullptr && env->runtime_deleted)) {
+    if (UNLIKELY(env != nullptr && env->IsRuntimeDeleted())) {
       CHECK(self->IsDaemon());
       // If the runtime has been deleted, then we cannot proceed. Just sleep forever. This may
       // occur for user daemon threads that get a spurious wakeup. This occurs for test 132 with
diff --git a/runtime/base/time_utils.h b/runtime/base/time_utils.h
index 919937f5ba..7648a109fa 100644
--- a/runtime/base/time_utils.h
+++ b/runtime/base/time_utils.h
@@ -76,6 +76,15 @@ static constexpr inline uint64_t MsToNs(uint64_t ms) {
   return ms * 1000 * 1000;
 }
 
+// Converts the given number of milliseconds to microseconds
+static constexpr inline uint64_t MsToUs(uint64_t ms) {
+  return ms * 1000;
+}
+
+static constexpr inline uint64_t UsToNs(uint64_t us) {
+  return us * 1000;
+}
+
 #if defined(__APPLE__)
 #ifndef CLOCK_REALTIME
 // No clocks to specify on OS/X < 10.12, fake value to pass to routines that require a clock.
diff --git a/runtime/check_jni.cc b/runtime/check_jni.cc
index 90f478f5f4..ce4cee827a 100644
--- a/runtime/check_jni.cc
+++ b/runtime/check_jni.cc
@@ -28,6 +28,7 @@
 #include "art_method-inl.h"
 #include "base/macros.h"
 #include "base/to_str.h"
+#include "base/time_utils.h"
 #include "class_linker-inl.h"
 #include "class_linker.h"
 #include "dex_file-inl.h"
@@ -55,24 +56,37 @@ using android::base::StringPrintf;
  * ===========================================================================
  */
 
+// Warn if a JNI critical is held for longer than 16ms.
+static constexpr uint64_t kCriticalWarnTimeUs = MsToUs(16);
+static_assert(kCriticalWarnTimeUs > 0, "No JNI critical warn time set");
+
 // Flags passed into ScopedCheck.
-#define kFlag_Default       0x0000
+static constexpr uint16_t kFlag_Default = 0x0000;
 
-#define kFlag_CritBad       0x0000      // Calling while in critical is not allowed.
-#define kFlag_CritOkay      0x0001      // Calling while in critical is allowed.
-#define kFlag_CritGet       0x0002      // This is a critical "get".
-#define kFlag_CritRelease   0x0003      // This is a critical "release".
-#define kFlag_CritMask      0x0003      // Bit mask to get "crit" value.
+// Calling while in critical is not allowed.
+static constexpr uint16_t kFlag_CritBad = 0x0000;
+// Calling while in critical is allowed.
+static constexpr uint16_t kFlag_CritOkay = 0x0001;
+// This is a critical "get".
+static constexpr uint16_t kFlag_CritGet = 0x0002;
+// This is a critical "release".
+static constexpr uint16_t kFlag_CritRelease = 0x0003;
+// Bit mask to get "crit" value.
+static constexpr uint16_t kFlag_CritMask = 0x0003;
 
-#define kFlag_ExcepBad      0x0000      // Raised exceptions are not allowed.
-#define kFlag_ExcepOkay     0x0004      // Raised exceptions are allowed.
+// Raised exceptions are allowed.
+static constexpr uint16_t kFlag_ExcepOkay = 0x0004;
 
-#define kFlag_Release       0x0010      // Are we in a non-critical release function?
-#define kFlag_NullableUtf   0x0020      // Are our UTF parameters nullable?
+// Are we in a non-critical release function?
+static constexpr uint16_t kFlag_Release = 0x0010;
+// Are our UTF parameters nullable?
+static constexpr uint16_t kFlag_NullableUtf = 0x0020;
 
-#define kFlag_Invocation    0x8000      // Part of the invocation interface (JavaVM*).
+// Part of the invocation interface (JavaVM*).
+static constexpr uint16_t kFlag_Invocation = 0x0100;
 
-#define kFlag_ForceTrace    0x80000000  // Add this to a JNI function's flags if you want to trace every call.
+// Add this to a JNI function's flags if you want to trace every call.
+static constexpr uint16_t kFlag_ForceTrace = 0x8000;
 
 class VarArgs;
 /*
@@ -249,8 +263,8 @@ class VarArgs {
 
 class ScopedCheck {
  public:
-  ScopedCheck(int flags, const char* functionName, bool has_method = true)
-      : function_name_(functionName), flags_(flags), indent_(0), has_method_(has_method) {
+  ScopedCheck(uint16_t flags, const char* functionName, bool has_method = true)
+      : function_name_(functionName), indent_(0), flags_(flags), has_method_(has_method) {
   }
 
   ~ScopedCheck() {}
@@ -1197,7 +1211,7 @@ class ScopedCheck {
     // this particular instance of JNIEnv.
     if (env != threadEnv) {
       // Get the thread owning the JNIEnv that's being used.
-      Thread* envThread = reinterpret_cast<JNIEnvExt*>(env)->self;
+      Thread* envThread = reinterpret_cast<JNIEnvExt*>(env)->self_;
       AbortF("thread %s using JNIEnv* from thread %s",
              ToStr<Thread>(*self).c_str(), ToStr<Thread>(*envThread).c_str());
       return false;
@@ -1209,7 +1223,7 @@ class ScopedCheck {
     case kFlag_CritOkay:    // okay to call this method
       break;
     case kFlag_CritBad:     // not okay to call
-      if (threadEnv->critical) {
+      if (threadEnv->critical_ > 0) {
         AbortF("thread %s using JNI after critical get",
                ToStr<Thread>(*self).c_str());
         return false;
@@ -1217,15 +1231,25 @@ class ScopedCheck {
       break;
     case kFlag_CritGet:     // this is a "get" call
       // Don't check here; we allow nested gets.
-      threadEnv->critical++;
+      if (threadEnv->critical_ == 0) {
+        threadEnv->critical_start_us_ = self->GetCpuMicroTime();
+      }
+      threadEnv->critical_++;
       break;
     case kFlag_CritRelease:  // this is a "release" call
-      threadEnv->critical--;
-      if (threadEnv->critical < 0) {
+      if (threadEnv->critical_ == 0) {
         AbortF("thread %s called too many critical releases",
                ToStr<Thread>(*self).c_str());
         return false;
+      } else if (threadEnv->critical_ == 1) {
+        // Leaving the critical region, possibly warn about long critical regions.
+        uint64_t critical_duration_us = self->GetCpuMicroTime() - threadEnv->critical_start_us_;
+        if (critical_duration_us > kCriticalWarnTimeUs) {
+          LOG(WARNING) << "JNI critical lock held for "
+                       << PrettyDuration(UsToNs(critical_duration_us)) << " on " << *self;
+        }
       }
+      threadEnv->critical_--;
       break;
     default:
       LOG(FATAL) << "Bad flags (internal error): " << flags_;
@@ -1357,9 +1381,10 @@ class ScopedCheck {
   // The name of the JNI function being checked.
   const char* const function_name_;
 
-  const int flags_;
   int indent_;
 
+  const uint16_t flags_;
+
   const bool has_method_;
 
   DISALLOW_COPY_AND_ASSIGN(ScopedCheck);
@@ -2592,11 +2617,11 @@ class CheckJNI {
 
  private:
   static JavaVMExt* GetJavaVMExt(JNIEnv* env) {
-    return reinterpret_cast<JNIEnvExt*>(env)->vm;
+    return reinterpret_cast<JNIEnvExt*>(env)->GetVm();
   }
 
   static const JNINativeInterface* baseEnv(JNIEnv* env) {
-    return reinterpret_cast<JNIEnvExt*>(env)->unchecked_functions;
+    return reinterpret_cast<JNIEnvExt*>(env)->unchecked_functions_;
   }
 
   static jobject NewRef(const char* function_name, JNIEnv* env, jobject obj, IndirectRefKind kind) {
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 55fa6328f5..ae285c7737 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -3391,7 +3391,7 @@ void ClassLinker::RegisterDexFileLocked(const DexFile& dex_file,
   // Clean up pass to remove null dex caches. Also check if we need to initialize OatFile .bss.
   // Null dex caches can occur due to class unloading and we are lazily removing null entries.
   bool initialize_oat_file_bss = (oat_file != nullptr);
-  JavaVMExt* const vm = self->GetJniEnv()->vm;
+  JavaVMExt* const vm = self->GetJniEnv()->GetVm();
   for (auto it = dex_caches_.begin(); it != dex_caches_.end(); ) {
     DexCacheData data = *it;
     if (self->IsJWeakCleared(data.weak_root)) {
@@ -5269,7 +5269,7 @@ void ClassLinker::RegisterClassLoader(ObjPtr<mirror::ClassLoader> class_loader)
   CHECK(class_loader->GetClassTable() == nullptr);
   Thread* const self = Thread::Current();
   ClassLoaderData data;
-  data.weak_root = self->GetJniEnv()->vm->AddWeakGlobalRef(self, class_loader);
+  data.weak_root = self->GetJniEnv()->GetVm()->AddWeakGlobalRef(self, class_loader);
   // Create and set the class table.
   data.class_table = new ClassTable;
   class_loader->SetClassTable(data.class_table);
diff --git a/runtime/entrypoints/quick/quick_jni_entrypoints.cc b/runtime/entrypoints/quick/quick_jni_entrypoints.cc
index b13b6fbcae..3c41a8c3b5 100644
--- a/runtime/entrypoints/quick/quick_jni_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_jni_entrypoints.cc
@@ -51,8 +51,8 @@ extern void ReadBarrierJni(mirror::CompressedReference<mirror::Object>* handle_o
 extern uint32_t JniMethodFastStart(Thread* self) {
   JNIEnvExt* env = self->GetJniEnv();
   DCHECK(env != nullptr);
-  uint32_t saved_local_ref_cookie = bit_cast<uint32_t>(env->local_ref_cookie);
-  env->local_ref_cookie = env->locals.GetSegmentState();
+  uint32_t saved_local_ref_cookie = bit_cast<uint32_t>(env->GetLocalRefCookie());
+  env->SetLocalRefCookie(env->GetLocalsSegmentState());
 
   if (kIsDebugBuild) {
     ArtMethod* native_method = *self->GetManagedStack()->GetTopQuickFrame();
@@ -66,8 +66,8 @@ extern uint32_t JniMethodFastStart(Thread* self) {
 extern uint32_t JniMethodStart(Thread* self) {
   JNIEnvExt* env = self->GetJniEnv();
   DCHECK(env != nullptr);
-  uint32_t saved_local_ref_cookie = bit_cast<uint32_t>(env->local_ref_cookie);
-  env->local_ref_cookie = env->locals.GetSegmentState();
+  uint32_t saved_local_ref_cookie = bit_cast<uint32_t>(env->GetLocalRefCookie());
+  env->SetLocalRefCookie(env->GetLocalsSegmentState());
   ArtMethod* native_method = *self->GetManagedStack()->GetTopQuickFrame();
   // TODO: Introduce special entrypoint for synchronized @FastNative methods?
   //       Or ban synchronized @FastNative outright to avoid the extra check here?
@@ -115,11 +115,11 @@ ALWAYS_INLINE static inline void GoToRunnableFast(Thread* self) {
 static void PopLocalReferences(uint32_t saved_local_ref_cookie, Thread* self)
     REQUIRES_SHARED(Locks::mutator_lock_) {
   JNIEnvExt* env = self->GetJniEnv();
-  if (UNLIKELY(env->check_jni)) {
+  if (UNLIKELY(env->IsCheckJniEnabled())) {
     env->CheckNoHeldMonitors();
   }
-  env->locals.SetSegmentState(env->local_ref_cookie);
-  env->local_ref_cookie = bit_cast<IRTSegmentState>(saved_local_ref_cookie);
+  env->SetLocalSegmentState(env->GetLocalRefCookie());
+  env->SetLocalRefCookie(bit_cast<IRTSegmentState>(saved_local_ref_cookie));
   self->PopHandleScope();
 }
 
@@ -156,7 +156,7 @@ static mirror::Object* JniMethodEndWithReferenceHandleResult(jobject result,
   }
   PopLocalReferences(saved_local_ref_cookie, self);
   // Process result.
-  if (UNLIKELY(self->GetJniEnv()->check_jni)) {
+  if (UNLIKELY(self->GetJniEnv()->IsCheckJniEnabled())) {
     // CheckReferenceResult can resolve types.
     StackHandleScope<1> hs(self);
     HandleWrapperObjPtr<mirror::Object> h_obj(hs.NewHandleWrapper(&o));
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index f7be4c8d5a..3ba12ca493 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -1299,7 +1299,7 @@ class TrimIndirectReferenceTableClosure : public Closure {
   explicit TrimIndirectReferenceTableClosure(Barrier* barrier) : barrier_(barrier) {
   }
   virtual void Run(Thread* thread) OVERRIDE NO_THREAD_SAFETY_ANALYSIS {
-    thread->GetJniEnv()->locals.Trim();
+    thread->GetJniEnv()->TrimLocals();
     // If thread is a running mutator, then act on behalf of the trim thread.
     // See the code in ThreadList::RunCheckpoint.
     barrier_->Pass(Thread::Current());
diff --git a/runtime/gc/reference_processor.cc b/runtime/gc/reference_processor.cc
index d58d09c794..c59642fe4e 100644
--- a/runtime/gc/reference_processor.cc
+++ b/runtime/gc/reference_processor.cc
@@ -273,7 +273,7 @@ void ReferenceProcessor::EnqueueClearedReferences(Thread* self) {
       jobject cleared_references;
       {
         ReaderMutexLock mu(self, *Locks::mutator_lock_);
-        cleared_references = self->GetJniEnv()->vm->AddGlobalRef(
+        cleared_references = self->GetJniEnv()->GetVm()->AddGlobalRef(
             self, cleared_references_.GetList());
       }
       if (kAsyncReferenceQueueAdd) {
diff --git a/runtime/indirect_reference_table.cc b/runtime/indirect_reference_table.cc
index 2c8ec47492..51878312d9 100644
--- a/runtime/indirect_reference_table.cc
+++ b/runtime/indirect_reference_table.cc
@@ -357,7 +357,7 @@ bool IndirectReferenceTable::Remove(IRTSegmentState previous_state, IndirectRef
     if (self->HandleScopeContains(reinterpret_cast<jobject>(iref))) {
       auto* env = self->GetJniEnv();
       DCHECK(env != nullptr);
-      if (env->check_jni) {
+      if (env->IsCheckJniEnabled()) {
         ScopedObjectAccess soa(self);
         LOG(WARNING) << "Attempt to remove non-JNI local reference, dumping thread";
         if (kDumpStackOnNonLocalReference) {
diff --git a/runtime/java_vm_ext.cc b/runtime/java_vm_ext.cc
index f8b82ed313..104fb66ed8 100644
--- a/runtime/java_vm_ext.cc
+++ b/runtime/java_vm_ext.cc
@@ -337,7 +337,7 @@ class Libraries {
       } else {
         VLOG(jni) << "[JNI_OnUnload found for \"" << library->GetPath() << "\"]: Calling...";
         JNI_OnUnloadFn jni_on_unload = reinterpret_cast<JNI_OnUnloadFn>(sym);
-        jni_on_unload(self->GetJniEnv()->vm, nullptr);
+        jni_on_unload(self->GetJniEnv()->GetVm(), nullptr);
       }
       delete library;
     }
diff --git a/runtime/jni_env_ext-inl.h b/runtime/jni_env_ext-inl.h
index d66df081c6..14f708b18d 100644
--- a/runtime/jni_env_ext-inl.h
+++ b/runtime/jni_env_ext-inl.h
@@ -26,7 +26,7 @@ namespace art {
 template<typename T>
 inline T JNIEnvExt::AddLocalReference(ObjPtr<mirror::Object> obj) {
   std::string error_msg;
-  IndirectRef ref = locals.Add(local_ref_cookie, obj, &error_msg);
+  IndirectRef ref = locals_.Add(local_ref_cookie_, obj, &error_msg);
   if (UNLIKELY(ref == nullptr)) {
     // This is really unexpected if we allow resizing local IRTs...
     LOG(FATAL) << error_msg;
@@ -35,10 +35,10 @@ inline T JNIEnvExt::AddLocalReference(ObjPtr<mirror::Object> obj) {
 
   // TODO: fix this to understand PushLocalFrame, so we can turn it on.
   if (false) {
-    if (check_jni) {
-      size_t entry_count = locals.Capacity();
+    if (check_jni_) {
+      size_t entry_count = locals_.Capacity();
       if (entry_count > 16) {
-        locals.Dump(LOG_STREAM(WARNING) << "Warning: more than 16 JNI local references: "
+        locals_.Dump(LOG_STREAM(WARNING) << "Warning: more than 16 JNI local references: "
                                         << entry_count << " (most recent was a "
                                         << mirror::Object::PrettyTypeOf(obj) << ")\n");
       // TODO: LOG(FATAL) in a later release?
diff --git a/runtime/jni_env_ext.cc b/runtime/jni_env_ext.cc
index 8352657f28..efe43ee0e9 100644
--- a/runtime/jni_env_ext.cc
+++ b/runtime/jni_env_ext.cc
@@ -21,6 +21,7 @@
 
 #include "android-base/stringprintf.h"
 
+#include "base/to_str.h"
 #include "check_jni.h"
 #include "indirect_reference_table.h"
 #include "java_vm_ext.h"
@@ -28,6 +29,7 @@
 #include "lock_word.h"
 #include "mirror/object-inl.h"
 #include "nth_caller_visitor.h"
+#include "scoped_thread_state_change.h"
 #include "thread-current-inl.h"
 #include "thread_list.h"
 
@@ -40,14 +42,11 @@ static constexpr size_t kMonitorsMax = 4096;  // Arbitrary sanity check.
 
 const JNINativeInterface* JNIEnvExt::table_override_ = nullptr;
 
-// Checking "locals" requires the mutator lock, but at creation time we're really only interested
-// in validity, which isn't changing. To avoid grabbing the mutator lock, factored out and tagged
-// with NO_THREAD_SAFETY_ANALYSIS.
-static bool CheckLocalsValid(JNIEnvExt* in) NO_THREAD_SAFETY_ANALYSIS {
+bool JNIEnvExt::CheckLocalsValid(JNIEnvExt* in) NO_THREAD_SAFETY_ANALYSIS {
   if (in == nullptr) {
     return false;
   }
-  return in->locals.IsValid();
+  return in->locals_.IsValid();
 }
 
 jint JNIEnvExt::GetEnvHandler(JavaVMExt* vm, /*out*/void** env, jint version) {
@@ -73,23 +72,23 @@ JNIEnvExt* JNIEnvExt::Create(Thread* self_in, JavaVMExt* vm_in, std::string* err
 }
 
 JNIEnvExt::JNIEnvExt(Thread* self_in, JavaVMExt* vm_in, std::string* error_msg)
-    : self(self_in),
-      vm(vm_in),
-      local_ref_cookie(kIRTFirstSegment),
-      locals(kLocalsInitial, kLocal, IndirectReferenceTable::ResizableCapacity::kYes, error_msg),
-      check_jni(false),
-      runtime_deleted(false),
-      critical(0),
-      monitors("monitors", kMonitorsInitial, kMonitorsMax) {
+    : self_(self_in),
+      vm_(vm_in),
+      local_ref_cookie_(kIRTFirstSegment),
+      locals_(kLocalsInitial, kLocal, IndirectReferenceTable::ResizableCapacity::kYes, error_msg),
+      monitors_("monitors", kMonitorsInitial, kMonitorsMax),
+      critical_(0),
+      check_jni_(false),
+      runtime_deleted_(false) {
   MutexLock mu(Thread::Current(), *Locks::jni_function_table_lock_);
-  check_jni = vm->IsCheckJniEnabled();
-  functions = GetFunctionTable(check_jni);
-  unchecked_functions = GetJniNativeInterface();
+  check_jni_ = vm_in->IsCheckJniEnabled();
+  functions = GetFunctionTable(check_jni_);
+  unchecked_functions_ = GetJniNativeInterface();
 }
 
 void JNIEnvExt::SetFunctionsToRuntimeShutdownFunctions() {
   functions = GetRuntimeShutdownNativeInterface();
-  runtime_deleted = true;
+  runtime_deleted_ = true;
 }
 
 JNIEnvExt::~JNIEnvExt() {
@@ -100,7 +99,7 @@ jobject JNIEnvExt::NewLocalRef(mirror::Object* obj) {
     return nullptr;
   }
   std::string error_msg;
-  jobject ref = reinterpret_cast<jobject>(locals.Add(local_ref_cookie, obj, &error_msg));
+  jobject ref = reinterpret_cast<jobject>(locals_.Add(local_ref_cookie_, obj, &error_msg));
   if (UNLIKELY(ref == nullptr)) {
     // This is really unexpected if we allow resizing local IRTs...
     LOG(FATAL) << error_msg;
@@ -111,12 +110,12 @@ jobject JNIEnvExt::NewLocalRef(mirror::Object* obj) {
 
 void JNIEnvExt::DeleteLocalRef(jobject obj) {
   if (obj != nullptr) {
-    locals.Remove(local_ref_cookie, reinterpret_cast<IndirectRef>(obj));
+    locals_.Remove(local_ref_cookie_, reinterpret_cast<IndirectRef>(obj));
   }
 }
 
 void JNIEnvExt::SetCheckJniEnabled(bool enabled) {
-  check_jni = enabled;
+  check_jni_ = enabled;
   MutexLock mu(Thread::Current(), *Locks::jni_function_table_lock_);
   functions = GetFunctionTable(enabled);
   // Check whether this is a no-op because of override.
@@ -126,20 +125,20 @@ void JNIEnvExt::SetCheckJniEnabled(bool enabled) {
 }
 
 void JNIEnvExt::DumpReferenceTables(std::ostream& os) {
-  locals.Dump(os);
-  monitors.Dump(os);
+  locals_.Dump(os);
+  monitors_.Dump(os);
 }
 
 void JNIEnvExt::PushFrame(int capacity) {
-  DCHECK_GE(locals.FreeCapacity(), static_cast<size_t>(capacity));
-  stacked_local_ref_cookies.push_back(local_ref_cookie);
-  local_ref_cookie = locals.GetSegmentState();
+  DCHECK_GE(locals_.FreeCapacity(), static_cast<size_t>(capacity));
+  stacked_local_ref_cookies_.push_back(local_ref_cookie_);
+  local_ref_cookie_ = locals_.GetSegmentState();
 }
 
 void JNIEnvExt::PopFrame() {
-  locals.SetSegmentState(local_ref_cookie);
-  local_ref_cookie = stacked_local_ref_cookies.back();
-  stacked_local_ref_cookies.pop_back();
+  locals_.SetSegmentState(local_ref_cookie_);
+  local_ref_cookie_ = stacked_local_ref_cookies_.back();
+  stacked_local_ref_cookies_.pop_back();
 }
 
 // Note: the offset code is brittle, as we can't use OFFSETOF_MEMBER or offsetof easily. Thus, there
@@ -188,7 +187,7 @@ static uintptr_t GetJavaCallFrame(Thread* self) REQUIRES_SHARED(Locks::mutator_l
 }
 
 void JNIEnvExt::RecordMonitorEnter(jobject obj) {
-  locked_objects_.push_back(std::make_pair(GetJavaCallFrame(self), obj));
+  locked_objects_.push_back(std::make_pair(GetJavaCallFrame(self_), obj));
 }
 
 static std::string ComputeMonitorDescription(Thread* self,
@@ -230,7 +229,7 @@ static void RemoveMonitors(Thread* self,
 }
 
 void JNIEnvExt::CheckMonitorRelease(jobject obj) {
-  uintptr_t current_frame = GetJavaCallFrame(self);
+  uintptr_t current_frame = GetJavaCallFrame(self_);
   std::pair<uintptr_t, jobject> exact_pair = std::make_pair(current_frame, obj);
   auto it = std::find(locked_objects_.begin(), locked_objects_.end(), exact_pair);
   bool will_abort = false;
@@ -238,11 +237,11 @@ void JNIEnvExt::CheckMonitorRelease(jobject obj) {
     locked_objects_.erase(it);
   } else {
     // Check whether this monitor was locked in another JNI "session."
-    ObjPtr<mirror::Object> mirror_obj = self->DecodeJObject(obj);
+    ObjPtr<mirror::Object> mirror_obj = self_->DecodeJObject(obj);
     for (std::pair<uintptr_t, jobject>& pair : locked_objects_) {
-      if (self->DecodeJObject(pair.second) == mirror_obj) {
-        std::string monitor_descr = ComputeMonitorDescription(self, pair.second);
-        vm->JniAbortF("<JNI MonitorExit>",
+      if (self_->DecodeJObject(pair.second) == mirror_obj) {
+        std::string monitor_descr = ComputeMonitorDescription(self_, pair.second);
+        vm_->JniAbortF("<JNI MonitorExit>",
                       "Unlocking monitor that wasn't locked here: %s",
                       monitor_descr.c_str());
         will_abort = true;
@@ -255,26 +254,26 @@ void JNIEnvExt::CheckMonitorRelease(jobject obj) {
   // the monitors table, otherwise we may visit local objects in GC during abort (which won't be
   // valid anymore).
   if (will_abort) {
-    RemoveMonitors(self, current_frame, &monitors, &locked_objects_);
+    RemoveMonitors(self_, current_frame, &monitors_, &locked_objects_);
   }
 }
 
 void JNIEnvExt::CheckNoHeldMonitors() {
-  uintptr_t current_frame = GetJavaCallFrame(self);
   // The locked_objects_ are grouped by their stack frame component, as this enforces structured
   // locking, and the groups form a stack. So the current frame entries are at the end. Check
   // whether the vector is empty, and when there are elements, whether the last element belongs
   // to this call - this signals that there are unlocked monitors.
   if (!locked_objects_.empty()) {
+    uintptr_t current_frame = GetJavaCallFrame(self_);
     std::pair<uintptr_t, jobject>& pair = locked_objects_[locked_objects_.size() - 1];
     if (pair.first == current_frame) {
-      std::string monitor_descr = ComputeMonitorDescription(self, pair.second);
-      vm->JniAbortF("<JNI End>",
+      std::string monitor_descr = ComputeMonitorDescription(self_, pair.second);
+      vm_->JniAbortF("<JNI End>",
                     "Still holding a locked object on JNI end: %s",
                     monitor_descr.c_str());
       // When we abort, also make sure that any locks from the current "session" are removed from
       // the monitors table, otherwise we may visit local objects in GC during abort.
-      RemoveMonitors(self, current_frame, &monitors, &locked_objects_);
+      RemoveMonitors(self_, current_frame, &monitors_, &locked_objects_);
     } else if (kIsDebugBuild) {
       // Make sure there are really no other entries and our checking worked as expected.
       for (std::pair<uintptr_t, jobject>& check_pair : locked_objects_) {
@@ -282,12 +281,18 @@ void JNIEnvExt::CheckNoHeldMonitors() {
       }
     }
   }
+  // Ensure critical locks aren't held when returning to Java.
+  if (critical_ > 0) {
+    vm_->JniAbortF("<JNI End>",
+                  "Critical lock held when returning to Java on thread %s",
+                  ToStr<Thread>(*self_).c_str());
+  }
 }
 
 static void ThreadResetFunctionTable(Thread* thread, void* arg ATTRIBUTE_UNUSED)
     REQUIRES(Locks::jni_function_table_lock_) {
   JNIEnvExt* env = thread->GetJniEnv();
-  bool check_jni = env->check_jni;
+  bool check_jni = env->IsCheckJniEnabled();
   env->functions = JNIEnvExt::GetFunctionTable(check_jni);
 }
 
diff --git a/runtime/jni_env_ext.h b/runtime/jni_env_ext.h
index 2f6c5dc92a..0e8fd03057 100644
--- a/runtime/jni_env_ext.h
+++ b/runtime/jni_env_ext.h
@@ -37,10 +37,15 @@ class Object;
 // low enough that it forces sanity checks.
 static constexpr size_t kLocalsInitial = 512;
 
-struct JNIEnvExt : public JNIEnv {
+class JNIEnvExt : public JNIEnv {
+ public:
   // Creates a new JNIEnvExt. Returns null on error, in which case error_msg
   // will contain a description of the error.
   static JNIEnvExt* Create(Thread* self, JavaVMExt* vm, std::string* error_msg);
+  static Offset SegmentStateOffset(size_t pointer_size);
+  static Offset LocalRefCookieOffset(size_t pointer_size);
+  static Offset SelfOffset(size_t pointer_size);
+  static jint GetEnvHandler(JavaVMExt* vm, /*out*/void** out, jint version);
 
   ~JNIEnvExt();
 
@@ -58,43 +63,44 @@ struct JNIEnvExt : public JNIEnv {
       REQUIRES_SHARED(Locks::mutator_lock_)
       REQUIRES(!Locks::alloc_tracker_lock_);
 
-  static Offset SegmentStateOffset(size_t pointer_size);
-  static Offset LocalRefCookieOffset(size_t pointer_size);
-  static Offset SelfOffset(size_t pointer_size);
-
-  static jint GetEnvHandler(JavaVMExt* vm, /*out*/void** out, jint version);
+  void UpdateLocal(IndirectRef iref, ObjPtr<mirror::Object> obj) REQUIRES_SHARED(Locks::mutator_lock_) {
+    locals_.Update(iref, obj);
+  }
 
   jobject NewLocalRef(mirror::Object* obj) REQUIRES_SHARED(Locks::mutator_lock_);
   void DeleteLocalRef(jobject obj) REQUIRES_SHARED(Locks::mutator_lock_);
 
-  Thread* const self;
-  JavaVMExt* const vm;
-
-  // Cookie used when using the local indirect reference table.
-  IRTSegmentState local_ref_cookie;
+  void TrimLocals() REQUIRES_SHARED(Locks::mutator_lock_) {
+    locals_.Trim();
+  }
+  void AssertLocalsEmpty() REQUIRES_SHARED(Locks::mutator_lock_) {
+    locals_.AssertEmpty();
+  }
+  size_t GetLocalsCapacity() REQUIRES_SHARED(Locks::mutator_lock_) {
+    return locals_.Capacity();
+  }
 
-  // JNI local references.
-  IndirectReferenceTable locals GUARDED_BY(Locks::mutator_lock_);
+  IRTSegmentState GetLocalRefCookie() const { return local_ref_cookie_; }
+  void SetLocalRefCookie(IRTSegmentState new_cookie) { local_ref_cookie_ = new_cookie; }
 
-  // Stack of cookies corresponding to PushLocalFrame/PopLocalFrame calls.
-  // TODO: to avoid leaks (and bugs), we need to clear this vector on entry (or return)
-  // to a native method.
-  std::vector<IRTSegmentState> stacked_local_ref_cookies;
+  IRTSegmentState GetLocalsSegmentState() const REQUIRES_SHARED(Locks::mutator_lock_) {
+    return locals_.GetSegmentState();
+  }
+  void SetLocalSegmentState(IRTSegmentState new_state) REQUIRES_SHARED(Locks::mutator_lock_) {
+    locals_.SetSegmentState(new_state);
+  }
 
-  // Frequently-accessed fields cached from JavaVM.
-  bool check_jni;
+  void VisitJniLocalRoots(RootVisitor* visitor, const RootInfo& root_info)
+      REQUIRES_SHARED(Locks::mutator_lock_) {
+    locals_.VisitRoots(visitor, root_info);
+  }
 
-  // If we are a JNI env for a daemon thread with a deleted runtime.
-  bool runtime_deleted;
+  Thread* GetSelf() const { return self_; }
+  JavaVMExt* GetVm() const { return vm_; }
 
-  // How many nested "critical" JNI calls are we in?
-  int critical;
+  bool IsRuntimeDeleted() const { return runtime_deleted_; }
+  bool IsCheckJniEnabled() const { return check_jni_; }
 
-  // Entered JNI monitors, for bulk exit on thread detach.
-  ReferenceTable monitors;
-
-  // Used by -Xcheck:jni.
-  const JNINativeInterface* unchecked_functions;
 
   // Functions to keep track of monitor lock and unlock operations. Used to ensure proper locking
   // rules in CheckJNI mode.
@@ -108,6 +114,11 @@ struct JNIEnvExt : public JNIEnv {
   // Check that no monitors are held that have been acquired in this JNI "segment."
   void CheckNoHeldMonitors() REQUIRES_SHARED(Locks::mutator_lock_);
 
+  void VisitMonitorRoots(RootVisitor* visitor, const RootInfo& root_info)
+      REQUIRES_SHARED(Locks::mutator_lock_) {
+    monitors_.VisitRoots(visitor, root_info);
+  }
+
   // Set the functions to the runtime shutdown functions.
   void SetFunctionsToRuntimeShutdownFunctions();
 
@@ -124,6 +135,11 @@ struct JNIEnvExt : public JNIEnv {
       REQUIRES(Locks::jni_function_table_lock_);
 
  private:
+  // Checking "locals" requires the mutator lock, but at creation time we're
+  // really only interested in validity, which isn't changing. To avoid grabbing
+  // the mutator lock, factored out and tagged with NO_THREAD_SAFETY_ANALYSIS.
+  static bool CheckLocalsValid(JNIEnvExt* in) NO_THREAD_SAFETY_ANALYSIS;
+
   // Override of function tables. This applies to both default as well as instrumented (CheckJNI)
   // function tables.
   static const JNINativeInterface* table_override_ GUARDED_BY(Locks::jni_function_table_lock_);
@@ -133,29 +149,73 @@ struct JNIEnvExt : public JNIEnv {
   JNIEnvExt(Thread* self, JavaVMExt* vm, std::string* error_msg)
       REQUIRES(!Locks::jni_function_table_lock_);
 
+  // Link to Thread::Current().
+  Thread* const self_;
+
+  // The invocation interface JavaVM.
+  JavaVMExt* const vm_;
+
+  // Cookie used when using the local indirect reference table.
+  IRTSegmentState local_ref_cookie_;
+
+  // JNI local references.
+  IndirectReferenceTable locals_ GUARDED_BY(Locks::mutator_lock_);
+
+  // Stack of cookies corresponding to PushLocalFrame/PopLocalFrame calls.
+  // TODO: to avoid leaks (and bugs), we need to clear this vector on entry (or return)
+  // to a native method.
+  std::vector<IRTSegmentState> stacked_local_ref_cookies_;
+
+  // Entered JNI monitors, for bulk exit on thread detach.
+  ReferenceTable monitors_;
+
+  // Used by -Xcheck:jni.
+  const JNINativeInterface* unchecked_functions_;
+
   // All locked objects, with the (Java caller) stack frame that locked them. Used in CheckJNI
   // to ensure that only monitors locked in this native frame are being unlocked, and that at
   // the end all are unlocked.
   std::vector<std::pair<uintptr_t, jobject>> locked_objects_;
+
+  // Start time of "critical" JNI calls to ensure that their use doesn't
+  // excessively block the VM with CheckJNI.
+  uint64_t critical_start_us_;
+
+  // How many nested "critical" JNI calls are we in? Used by CheckJNI to ensure that criticals are
+  uint32_t critical_;
+
+  // Frequently-accessed fields cached from JavaVM.
+  bool check_jni_;
+
+  // If we are a JNI env for a daemon thread with a deleted runtime.
+  bool runtime_deleted_;
+
+  friend class CheckJNI;
+  friend class JNI;
+  friend class ScopedCheck;
+  friend class ScopedJniEnvLocalRefState;
+  friend class Thread;
+  ART_FRIEND_TEST(JniInternalTest, JNIEnvExtOffsets);
 };
 
 // Used to save and restore the JNIEnvExt state when not going through code created by the JNI
 // compiler.
 class ScopedJniEnvLocalRefState {
  public:
-  explicit ScopedJniEnvLocalRefState(JNIEnvExt* env) : env_(env) {
-    saved_local_ref_cookie_ = env->local_ref_cookie;
-    env->local_ref_cookie = env->locals.GetSegmentState();
+  explicit ScopedJniEnvLocalRefState(JNIEnvExt* env) :
+      env_(env),
+      saved_local_ref_cookie_(env->local_ref_cookie_) {
+    env->local_ref_cookie_ = env->locals_.GetSegmentState();
   }
 
   ~ScopedJniEnvLocalRefState() {
-    env_->locals.SetSegmentState(env_->local_ref_cookie);
-    env_->local_ref_cookie = saved_local_ref_cookie_;
+    env_->locals_.SetSegmentState(env_->local_ref_cookie_);
+    env_->local_ref_cookie_ = saved_local_ref_cookie_;
   }
 
  private:
   JNIEnvExt* const env_;
-  IRTSegmentState saved_local_ref_cookie_;
+  const IRTSegmentState saved_local_ref_cookie_;
 
   DISALLOW_COPY_AND_ASSIGN(ScopedJniEnvLocalRefState);
 };
diff --git a/runtime/jni_internal.cc b/runtime/jni_internal.cc
index 48fc5f730b..53ae628a44 100644
--- a/runtime/jni_internal.cc
+++ b/runtime/jni_internal.cc
@@ -383,7 +383,7 @@ int ThrowNewException(JNIEnv* env, jclass exception_class, const char* msg, jobj
 }
 
 static JavaVMExt* JavaVmExtFromEnv(JNIEnv* env) {
-  return reinterpret_cast<JNIEnvExt*>(env)->vm;
+  return reinterpret_cast<JNIEnvExt*>(env)->GetVm();
 }
 
 #define CHECK_NON_NULL_ARGUMENT(value) \
@@ -545,7 +545,7 @@ class JNI {
   }
 
   static jboolean ExceptionCheck(JNIEnv* env) {
-    return static_cast<JNIEnvExt*>(env)->self->IsExceptionPending() ? JNI_TRUE : JNI_FALSE;
+    return static_cast<JNIEnvExt*>(env)->self_->IsExceptionPending() ? JNI_TRUE : JNI_FALSE;
   }
 
   static void ExceptionClear(JNIEnv* env) {
@@ -623,8 +623,8 @@ class JNI {
   }
 
   static void DeleteGlobalRef(JNIEnv* env, jobject obj) {
-    JavaVMExt* vm = down_cast<JNIEnvExt*>(env)->vm;
-    Thread* self = down_cast<JNIEnvExt*>(env)->self;
+    JavaVMExt* vm = down_cast<JNIEnvExt*>(env)->GetVm();
+    Thread* self = down_cast<JNIEnvExt*>(env)->self_;
     vm->DeleteGlobalRef(self, obj);
   }
 
@@ -635,8 +635,8 @@ class JNI {
   }
 
   static void DeleteWeakGlobalRef(JNIEnv* env, jweak obj) {
-    JavaVMExt* vm = down_cast<JNIEnvExt*>(env)->vm;
-    Thread* self = down_cast<JNIEnvExt*>(env)->self;
+    JavaVMExt* vm = down_cast<JNIEnvExt*>(env)->GetVm();
+    Thread* self = down_cast<JNIEnvExt*>(env)->self_;
     vm->DeleteWeakGlobalRef(self, obj);
   }
 
@@ -659,7 +659,7 @@ class JNI {
     // it. b/22119403
     ScopedObjectAccess soa(env);
     auto* ext_env = down_cast<JNIEnvExt*>(env);
-    if (!ext_env->locals.Remove(ext_env->local_ref_cookie, obj)) {
+    if (!ext_env->locals_.Remove(ext_env->local_ref_cookie_, obj)) {
       // Attempting to delete a local reference that is not in the
       // topmost local reference frame is a no-op.  DeleteLocalRef returns
       // void and doesn't throw any exceptions, but we should probably
@@ -2310,7 +2310,7 @@ class JNI {
       // first, either as a direct or a virtual method. Then move to
       // the parent.
       ArtMethod* m = nullptr;
-      bool warn_on_going_to_parent = down_cast<JNIEnvExt*>(env)->vm->IsCheckJniEnabled();
+      bool warn_on_going_to_parent = down_cast<JNIEnvExt*>(env)->GetVm()->IsCheckJniEnabled();
       for (ObjPtr<mirror::Class> current_class = c.Get();
            current_class != nullptr;
            current_class = current_class->GetSuperClass()) {
@@ -2399,7 +2399,7 @@ class JNI {
     ObjPtr<mirror::Object> o = soa.Decode<mirror::Object>(java_object);
     o = o->MonitorEnter(soa.Self());
     if (soa.Self()->HoldsLock(o)) {
-      soa.Env()->monitors.Add(o);
+      soa.Env()->monitors_.Add(o);
     }
     if (soa.Self()->IsExceptionPending()) {
       return JNI_ERR;
@@ -2414,7 +2414,7 @@ class JNI {
     bool remove_mon = soa.Self()->HoldsLock(o);
     o->MonitorExit(soa.Self());
     if (remove_mon) {
-      soa.Env()->monitors.Remove(o);
+      soa.Env()->monitors_.Remove(o);
     }
     if (soa.Self()->IsExceptionPending()) {
       return JNI_ERR;
@@ -2458,7 +2458,7 @@ class JNI {
     jobject result = env->NewObject(WellKnownClasses::java_nio_DirectByteBuffer,
                                     WellKnownClasses::java_nio_DirectByteBuffer_init,
                                     address_arg, capacity_arg);
-    return static_cast<JNIEnvExt*>(env)->self->IsExceptionPending() ? nullptr : result;
+    return static_cast<JNIEnvExt*>(env)->self_->IsExceptionPending() ? nullptr : result;
   }
 
   static void* GetDirectBufferAddress(JNIEnv* env, jobject java_buffer) {
@@ -2504,7 +2504,7 @@ class JNI {
     }
 
     std::string error_msg;
-    if (!soa.Env()->locals.EnsureFreeCapacity(static_cast<size_t>(desired_capacity), &error_msg)) {
+    if (!soa.Env()->locals_.EnsureFreeCapacity(static_cast<size_t>(desired_capacity), &error_msg)) {
       std::string caller_error = android::base::StringPrintf("%s: %s", caller, error_msg.c_str());
       soa.Self()->ThrowOutOfMemoryError(caller_error.c_str());
       return JNI_ERR;
diff --git a/runtime/jni_internal_test.cc b/runtime/jni_internal_test.cc
index efeff0a378..ad24c94ae9 100644
--- a/runtime/jni_internal_test.cc
+++ b/runtime/jni_internal_test.cc
@@ -867,7 +867,7 @@ TEST_F(JniInternalTest, GetStaticMethodID) {
 
 static size_t GetLocalsCapacity(JNIEnv* env) {
   ScopedObjectAccess soa(Thread::Current());
-  return reinterpret_cast<JNIEnvExt*>(env)->locals.Capacity();
+  return reinterpret_cast<JNIEnvExt*>(env)->GetLocalsCapacity();
 }
 
 TEST_F(JniInternalTest, FromReflectedField_ToReflectedField) {
@@ -2400,15 +2400,15 @@ TEST_F(JniInternalTest, IndirectReferenceTableOffsets) {
 
 // Test the offset computation of JNIEnvExt offsets. b/26071368.
 TEST_F(JniInternalTest, JNIEnvExtOffsets) {
-  EXPECT_EQ(OFFSETOF_MEMBER(JNIEnvExt, local_ref_cookie),
+  EXPECT_EQ(OFFSETOF_MEMBER(JNIEnvExt, local_ref_cookie_),
             JNIEnvExt::LocalRefCookieOffset(sizeof(void*)).Uint32Value());
 
-  EXPECT_EQ(OFFSETOF_MEMBER(JNIEnvExt, self), JNIEnvExt::SelfOffset(sizeof(void*)).Uint32Value());
+  EXPECT_EQ(OFFSETOF_MEMBER(JNIEnvExt, self_), JNIEnvExt::SelfOffset(sizeof(void*)).Uint32Value());
 
   // segment_state_ is private in the IndirectReferenceTable. So this test isn't as good as we'd
   // hope it to be.
   uint32_t segment_state_now =
-      OFFSETOF_MEMBER(JNIEnvExt, locals) +
+      OFFSETOF_MEMBER(JNIEnvExt, locals_) +
       IndirectReferenceTable::SegmentStateOffset(sizeof(void*)).Uint32Value();
   uint32_t segment_state_computed = JNIEnvExt::SegmentStateOffset(sizeof(void*)).Uint32Value();
   EXPECT_EQ(segment_state_now, segment_state_computed);
diff --git a/runtime/native/dalvik_system_VMRuntime.cc b/runtime/native/dalvik_system_VMRuntime.cc
index 1b5c535c87..6446e1b4f5 100644
--- a/runtime/native/dalvik_system_VMRuntime.cc
+++ b/runtime/native/dalvik_system_VMRuntime.cc
@@ -223,7 +223,7 @@ static jboolean VMRuntime_is64Bit(JNIEnv*, jobject) {
 }
 
 static jboolean VMRuntime_isCheckJniEnabled(JNIEnv* env, jobject) {
-  return down_cast<JNIEnvExt*>(env)->vm->IsCheckJniEnabled() ? JNI_TRUE : JNI_FALSE;
+  return down_cast<JNIEnvExt*>(env)->GetVm()->IsCheckJniEnabled() ? JNI_TRUE : JNI_FALSE;
 }
 
 static void VMRuntime_setTargetSdkVersionNative(JNIEnv*, jobject, jint target_sdk_version) {
diff --git a/runtime/native/java_lang_Thread.cc b/runtime/native/java_lang_Thread.cc
index a717264bcb..9a52f7002b 100644
--- a/runtime/native/java_lang_Thread.cc
+++ b/runtime/native/java_lang_Thread.cc
@@ -37,7 +37,7 @@ static jobject Thread_currentThread(JNIEnv* env, jclass) {
 }
 
 static jboolean Thread_interrupted(JNIEnv* env, jclass) {
-  return static_cast<JNIEnvExt*>(env)->self->Interrupted() ? JNI_TRUE : JNI_FALSE;
+  return static_cast<JNIEnvExt*>(env)->GetSelf()->Interrupted() ? JNI_TRUE : JNI_FALSE;
 }
 
 static jboolean Thread_isInterrupted(JNIEnv* env, jobject java_thread) {
diff --git a/runtime/native/org_apache_harmony_dalvik_ddmc_DdmVmInternal.cc b/runtime/native/org_apache_harmony_dalvik_ddmc_DdmVmInternal.cc
index 7b733824c5..836637f4f0 100644
--- a/runtime/native/org_apache_harmony_dalvik_ddmc_DdmVmInternal.cc
+++ b/runtime/native/org_apache_harmony_dalvik_ddmc_DdmVmInternal.cc
@@ -32,6 +32,10 @@
 
 namespace art {
 
+static Thread* GetSelf(JNIEnv* env) {
+  return static_cast<JNIEnvExt*>(env)->GetSelf();
+}
+
 static void DdmVmInternal_enableRecentAllocations(JNIEnv*, jclass, jboolean enable) {
   Dbg::SetAllocTrackingEnabled(enable);
 }
@@ -51,7 +55,7 @@ static jboolean DdmVmInternal_getRecentAllocationStatus(JNIEnv*, jclass) {
  */
 static jobjectArray DdmVmInternal_getStackTraceById(JNIEnv* env, jclass, jint thin_lock_id) {
   jobjectArray trace = nullptr;
-  Thread* const self = Thread::Current();
+  Thread* const self = GetSelf(env);
   if (static_cast<uint32_t>(thin_lock_id) == self->GetThreadId()) {
     // No need to suspend ourself to build stacktrace.
     ScopedObjectAccess soa(env);
@@ -136,7 +140,7 @@ static void ThreadStatsGetterCallback(Thread* t, void* context) {
 
 static jbyteArray DdmVmInternal_getThreadStats(JNIEnv* env, jclass) {
   std::vector<uint8_t> bytes;
-  Thread* self = static_cast<JNIEnvExt*>(env)->self;
+  Thread* self = GetSelf(env);
   {
     MutexLock mu(self, *Locks::thread_list_lock_);
     ThreadList* thread_list = Runtime::Current()->GetThreadList();
diff --git a/runtime/reflection.cc b/runtime/reflection.cc
index 9683cedd4d..cacbe87f71 100644
--- a/runtime/reflection.cc
+++ b/runtime/reflection.cc
@@ -447,7 +447,7 @@ static void InvokeWithArgArray(const ScopedObjectAccessAlreadyRunnable& soa,
                                const char* shorty)
     REQUIRES_SHARED(Locks::mutator_lock_) {
   uint32_t* args = arg_array->GetArray();
-  if (UNLIKELY(soa.Env()->check_jni)) {
+  if (UNLIKELY(soa.Env()->IsCheckJniEnabled())) {
     CheckMethodArguments(soa.Vm(), method->GetInterfaceMethodIfProxy(kRuntimePointerSize), args);
   }
   method->Invoke(soa.Self(), args, arg_array->GetNumBytes(), result, shorty);
@@ -927,14 +927,14 @@ void UpdateReference(Thread* self, jobject obj, ObjPtr<mirror::Object> result) {
   IndirectRef ref = reinterpret_cast<IndirectRef>(obj);
   IndirectRefKind kind = IndirectReferenceTable::GetIndirectRefKind(ref);
   if (kind == kLocal) {
-    self->GetJniEnv()->locals.Update(obj, result);
+    self->GetJniEnv()->UpdateLocal(obj, result);
   } else if (kind == kHandleScopeOrInvalid) {
     LOG(FATAL) << "Unsupported UpdateReference for kind kHandleScopeOrInvalid";
   } else if (kind == kGlobal) {
-    self->GetJniEnv()->vm->UpdateGlobal(self, ref, result);
+    self->GetJniEnv()->GetVm()->UpdateGlobal(self, ref, result);
   } else {
     DCHECK_EQ(kind, kWeakGlobal);
-    self->GetJniEnv()->vm->UpdateWeakGlobal(self, ref, result);
+    self->GetJniEnv()->GetVm()->UpdateWeakGlobal(self, ref, result);
   }
 }
 
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index a172392c51..54aa9e57e6 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -807,7 +807,7 @@ bool Runtime::Start() {
 
   {
     ScopedObjectAccess soa(self);
-    self->GetJniEnv()->locals.AssertEmpty();
+    self->GetJniEnv()->AssertLocalsEmpty();
   }
 
   VLOG(startup) << "Runtime::Start exiting";
diff --git a/runtime/scoped_thread_state_change-inl.h b/runtime/scoped_thread_state_change-inl.h
index a9702a70bc..f95593209b 100644
--- a/runtime/scoped_thread_state_change-inl.h
+++ b/runtime/scoped_thread_state_change-inl.h
@@ -97,12 +97,12 @@ inline bool ScopedObjectAccessAlreadyRunnable::IsRunnable() const {
 }
 
 inline ScopedObjectAccessAlreadyRunnable::ScopedObjectAccessAlreadyRunnable(JNIEnv* env)
-    : self_(ThreadForEnv(env)), env_(down_cast<JNIEnvExt*>(env)), vm_(env_->vm) {}
+    : self_(ThreadForEnv(env)), env_(down_cast<JNIEnvExt*>(env)), vm_(env_->GetVm()) {}
 
 inline ScopedObjectAccessAlreadyRunnable::ScopedObjectAccessAlreadyRunnable(Thread* self)
     : self_(self),
       env_(down_cast<JNIEnvExt*>(self->GetJniEnv())),
-      vm_(env_ != nullptr ? env_->vm : nullptr) {}
+      vm_(env_ != nullptr ? env_->GetVm() : nullptr) {}
 
 inline ScopedObjectAccessUnchecked::ScopedObjectAccessUnchecked(JNIEnv* env)
     : ScopedObjectAccessAlreadyRunnable(env), tsc_(Self(), kRunnable) {
diff --git a/runtime/scoped_thread_state_change.h b/runtime/scoped_thread_state_change.h
index 02b6124118..0c42c5ae8d 100644
--- a/runtime/scoped_thread_state_change.h
+++ b/runtime/scoped_thread_state_change.h
@@ -27,7 +27,7 @@
 namespace art {
 
 class JavaVMExt;
-struct JNIEnvExt;
+class JNIEnvExt;
 template<class MirrorType> class ObjPtr;
 class Thread;
 
diff --git a/runtime/thread-inl.h b/runtime/thread-inl.h
index 62b0789a43..7392dcf6a5 100644
--- a/runtime/thread-inl.h
+++ b/runtime/thread-inl.h
@@ -34,7 +34,7 @@ namespace art {
 // Quickly access the current thread from a JNIEnv.
 static inline Thread* ThreadForEnv(JNIEnv* env) {
   JNIEnvExt* full_env(down_cast<JNIEnvExt*>(env));
-  return full_env->self;
+  return full_env->GetSelf();
 }
 
 inline void Thread::AllowThreadSuspension() {
diff --git a/runtime/thread.cc b/runtime/thread.cc
index b86e56b531..5777d141fa 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -621,7 +621,7 @@ void Thread::InstallImplicitProtection() {
 
 void Thread::CreateNativeThread(JNIEnv* env, jobject java_peer, size_t stack_size, bool is_daemon) {
   CHECK(java_peer != nullptr);
-  Thread* self = static_cast<JNIEnvExt*>(env)->self;
+  Thread* self = static_cast<JNIEnvExt*>(env)->GetSelf();
 
   if (VLOG_IS_ON(threads)) {
     ScopedObjectAccess soa(env);
@@ -754,8 +754,8 @@ bool Thread::Init(ThreadList* thread_list, JavaVMExt* java_vm, JNIEnvExt* jni_en
   tls32_.thin_lock_thread_id = thread_list->AllocThreadId(this);
 
   if (jni_env_ext != nullptr) {
-    DCHECK_EQ(jni_env_ext->vm, java_vm);
-    DCHECK_EQ(jni_env_ext->self, this);
+    DCHECK_EQ(jni_env_ext->GetVm(), java_vm);
+    DCHECK_EQ(jni_env_ext->GetSelf(), this);
     tlsPtr_.jni_env = jni_env_ext;
   } else {
     std::string error_msg;
@@ -853,7 +853,7 @@ Thread* Thread::Attach(const char* thread_name,
       if (thread_name != nullptr) {
         self->tlsPtr_.name->assign(thread_name);
         ::art::SetThreadName(thread_name);
-      } else if (self->GetJniEnv()->check_jni) {
+      } else if (self->GetJniEnv()->IsCheckJniEnabled()) {
         LOG(WARNING) << *Thread::Current() << " attached without supplying a name";
       }
     }
@@ -2170,7 +2170,7 @@ void Thread::Destroy() {
       ScopedObjectAccess soa(self);
       MonitorExitVisitor visitor(self);
       // On thread detach, all monitors entered with JNI MonitorEnter are automatically exited.
-      tlsPtr_.jni_env->monitors.VisitRoots(&visitor, RootInfo(kRootVMInternal));
+      tlsPtr_.jni_env->monitors_.VisitRoots(&visitor, RootInfo(kRootVMInternal));
     }
     // Release locally held global references which releasing may require the mutator lock.
     if (tlsPtr_.jpeer != nullptr) {
@@ -2339,7 +2339,7 @@ ObjPtr<mirror::Object> Thread::DecodeJObject(jobject obj) const {
   bool expect_null = false;
   // The "kinds" below are sorted by the frequency we expect to encounter them.
   if (kind == kLocal) {
-    IndirectReferenceTable& locals = tlsPtr_.jni_env->locals;
+    IndirectReferenceTable& locals = tlsPtr_.jni_env->locals_;
     // Local references do not need a read barrier.
     result = locals.Get<kWithoutReadBarrier>(ref);
   } else if (kind == kHandleScopeOrInvalid) {
@@ -2350,15 +2350,15 @@ ObjPtr<mirror::Object> Thread::DecodeJObject(jobject obj) const {
       result = reinterpret_cast<StackReference<mirror::Object>*>(obj)->AsMirrorPtr();
       VerifyObject(result);
     } else {
-      tlsPtr_.jni_env->vm->JniAbortF(nullptr, "use of invalid jobject %p", obj);
+      tlsPtr_.jni_env->vm_->JniAbortF(nullptr, "use of invalid jobject %p", obj);
       expect_null = true;
       result = nullptr;
     }
   } else if (kind == kGlobal) {
-    result = tlsPtr_.jni_env->vm->DecodeGlobal(ref);
+    result = tlsPtr_.jni_env->vm_->DecodeGlobal(ref);
   } else {
     DCHECK_EQ(kind, kWeakGlobal);
-    result = tlsPtr_.jni_env->vm->DecodeWeakGlobal(const_cast<Thread*>(this), ref);
+    result = tlsPtr_.jni_env->vm_->DecodeWeakGlobal(const_cast<Thread*>(this), ref);
     if (Runtime::Current()->IsClearedJniWeakGlobal(result)) {
       // This is a special case where it's okay to return null.
       expect_null = true;
@@ -2367,7 +2367,7 @@ ObjPtr<mirror::Object> Thread::DecodeJObject(jobject obj) const {
   }
 
   if (UNLIKELY(!expect_null && result == nullptr)) {
-    tlsPtr_.jni_env->vm->JniAbortF(nullptr, "use of deleted %s %p",
+    tlsPtr_.jni_env->vm_->JniAbortF(nullptr, "use of deleted %s %p",
                                    ToStr<IndirectRefKind>(kind).c_str(), obj);
   }
   return result;
@@ -2378,7 +2378,7 @@ bool Thread::IsJWeakCleared(jweak obj) const {
   IndirectRef ref = reinterpret_cast<IndirectRef>(obj);
   IndirectRefKind kind = IndirectReferenceTable::GetIndirectRefKind(ref);
   CHECK_EQ(kind, kWeakGlobal);
-  return tlsPtr_.jni_env->vm->IsWeakGlobalCleared(const_cast<Thread*>(this), ref);
+  return tlsPtr_.jni_env->vm_->IsWeakGlobalCleared(const_cast<Thread*>(this), ref);
 }
 
 // Implements java.lang.Thread.interrupted.
@@ -3516,8 +3516,8 @@ void Thread::VisitRoots(RootVisitor* visitor) {
                        RootInfo(kRootNativeStack, thread_id));
   }
   visitor->VisitRootIfNonNull(&tlsPtr_.monitor_enter_object, RootInfo(kRootNativeStack, thread_id));
-  tlsPtr_.jni_env->locals.VisitRoots(visitor, RootInfo(kRootJNILocal, thread_id));
-  tlsPtr_.jni_env->monitors.VisitRoots(visitor, RootInfo(kRootJNIMonitor, thread_id));
+  tlsPtr_.jni_env->VisitJniLocalRoots(visitor, RootInfo(kRootJNILocal, thread_id));
+  tlsPtr_.jni_env->VisitMonitorRoots(visitor, RootInfo(kRootJNIMonitor, thread_id));
   HandleScopeVisitRoots(visitor, thread_id);
   if (tlsPtr_.debug_invoke_req != nullptr) {
     tlsPtr_.debug_invoke_req->VisitRoots(visitor, RootInfo(kRootDebugger, thread_id));
diff --git a/runtime/thread.h b/runtime/thread.h
index 0803975d26..0f1d7a4a8a 100644
--- a/runtime/thread.h
+++ b/runtime/thread.h
@@ -86,7 +86,7 @@ class DeoptimizationContextRecord;
 class DexFile;
 class FrameIdToShadowFrame;
 class JavaVMExt;
-struct JNIEnvExt;
+class JNIEnvExt;
 class Monitor;
 class RootVisitor;
 class ScopedObjectAccessAlreadyRunnable;
diff --git a/test/136-daemon-jni-shutdown/daemon_jni_shutdown.cc b/test/136-daemon-jni-shutdown/daemon_jni_shutdown.cc
index 7d40f5773d..f01b82553d 100644
--- a/test/136-daemon-jni-shutdown/daemon_jni_shutdown.cc
+++ b/test/136-daemon-jni-shutdown/daemon_jni_shutdown.cc
@@ -58,7 +58,7 @@ extern "C" JNIEXPORT void JNICALL Java_Main_destroyJavaVMAndExit(JNIEnv* env, jc
   Thread* const self = Thread::Current();
   self->SetTopOfStack(nullptr);
   self->SetTopOfShadowStack(nullptr);
-  JavaVM* vm = down_cast<JNIEnvExt*>(env)->vm;
+  JavaVM* vm = down_cast<JNIEnvExt*>(env)->GetVm();
   vm->DetachCurrentThread();
   // Open ourself again to make sure the native library does not get unloaded from
   // underneath us due to DestroyJavaVM. b/28406866