From 498b160f0cb61ea4756d8ce859ae73c522366458 Mon Sep 17 00:00:00 2001
From: Hiroshi Yamauchi <yamauchi@google.com>
Date: Wed, 16 Sep 2015 21:11:44 -0700
Subject: [PATCH] Allow null self only in DecodeWeakGlobalDuringShutdown().

To follow up CL 169855, allow a null current thread only in
DecodeWeakGlobalDuringShutdown() as a special case rather than
DecodeWeakGlobal(). This is to prevent a bug where null is accidentally
passed to DecodeWeakGlobal().

Bug: 23897251

Change-Id: I5e7bb78ec739b8bfcf77284ed321d507737ee33e
---
 runtime/class_linker.cc |  2 +-
 runtime/java_vm_ext.cc  | 23 +++++++++++++++++------
 runtime/java_vm_ext.h   |  6 ++++++
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 73da2cbe5..bc8a9f493 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -1505,7 +1505,7 @@ ClassLinker::~ClassLinker() {
   JavaVMExt* const vm = Runtime::Current()->GetJavaVM();
   for (jweak weak_root : class_loaders_) {
     auto* const class_loader = down_cast<mirror::ClassLoader*>(
-        vm->DecodeWeakGlobal(self, weak_root));
+        vm->DecodeWeakGlobalDuringShutdown(self, weak_root));
     if (class_loader != nullptr) {
       delete class_loader->GetClassTable();
     }
diff --git a/runtime/java_vm_ext.cc b/runtime/java_vm_ext.cc
index 63e8887ff..531e03926 100644
--- a/runtime/java_vm_ext.cc
+++ b/runtime/java_vm_ext.cc
@@ -580,12 +580,10 @@ inline bool JavaVMExt::MayAccessWeakGlobals(Thread* self) const {
 }
 
 inline bool JavaVMExt::MayAccessWeakGlobalsUnlocked(Thread* self) const {
-  if (kUseReadBarrier) {
-    // self can be null during a runtime shutdown. ~Runtime()->~ClassLinker()->DecodeWeakGlobal().
-    return self != nullptr ? self->GetWeakRefAccessEnabled() : true;
-  } else {
-    return allow_accessing_weak_globals_.LoadSequentiallyConsistent();
-  }
+  DCHECK(self != nullptr);
+  return kUseReadBarrier ?
+      self->GetWeakRefAccessEnabled() :
+      allow_accessing_weak_globals_.LoadSequentiallyConsistent();
 }
 
 mirror::Object* JavaVMExt::DecodeWeakGlobal(Thread* self, IndirectRef ref) {
@@ -613,6 +611,19 @@ mirror::Object* JavaVMExt::DecodeWeakGlobalLocked(Thread* self, IndirectRef ref)
   return weak_globals_.Get(ref);
 }
 
+mirror::Object* JavaVMExt::DecodeWeakGlobalDuringShutdown(Thread* self, IndirectRef ref) {
+  DCHECK_EQ(GetIndirectRefKind(ref), kWeakGlobal);
+  DCHECK(Runtime::Current()->IsShuttingDown(self));
+  if (self != nullptr) {
+    return DecodeWeakGlobal(self, ref);
+  }
+  // self can be null during a runtime shutdown. ~Runtime()->~ClassLinker()->DecodeWeakGlobal().
+  if (!kUseReadBarrier) {
+    DCHECK(allow_accessing_weak_globals_.LoadSequentiallyConsistent());
+  }
+  return weak_globals_.SynchronizedGet(ref);
+}
+
 void JavaVMExt::UpdateWeakGlobal(Thread* self, IndirectRef ref, mirror::Object* result) {
   MutexLock mu(self, weak_globals_lock_);
   weak_globals_.Update(ref, result);
diff --git a/runtime/java_vm_ext.h b/runtime/java_vm_ext.h
index 87430c800..b539bbdba 100644
--- a/runtime/java_vm_ext.h
+++ b/runtime/java_vm_ext.h
@@ -138,6 +138,12 @@ class JavaVMExt : public JavaVM {
       SHARED_REQUIRES(Locks::mutator_lock_)
       REQUIRES(weak_globals_lock_);
 
+  // Like DecodeWeakGlobal() but to be used only during a runtime shutdown where self may be
+  // null.
+  mirror::Object* DecodeWeakGlobalDuringShutdown(Thread* self, IndirectRef ref)
+      SHARED_REQUIRES(Locks::mutator_lock_)
+      REQUIRES(!weak_globals_lock_);
+
   Mutex& WeakGlobalsLock() RETURN_CAPABILITY(weak_globals_lock_) {
     return weak_globals_lock_;
   }
-- 
2.11.0