Allow OpenSSLPrivateKeyStore and Android global app context leaking.

base/android/jni_android.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/android/jni_android.h"
 
 #include <map>
 
 #include "base/android/build_info.h"
 #include "base/android/jni_string.h"
 #include "base/atomicops.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/threading/platform_thread.h"
 
 namespace {
 using base::android::GetClass;
 using base::android::GetMethodID;
 using base::android::ScopedJavaLocalRef;
 
-JavaVM* g_jvm = NULL;
-
-base::LazyInstance<base::android::ScopedJavaGlobalRef<jobject> >
-    g_application_context = LAZY_INSTANCE_INITIALIZER;
-
 struct MethodIdentifier {
   const char* class_name;
   const char* method;
   const char* jni_signature;
 
   bool operator<(const MethodIdentifier& other) const {
     int r = strcmp(class_name, other.class_name);
     if (r < 0) {
       return true;
     } else if (r > 0) {
       return false;
     }
 
     r = strcmp(method, other.method);
     if (r < 0) {
       return true;
     } else if (r > 0) {
       return false;
     }
 
     return strcmp(jni_signature, other.jni_signature) < 0;
   }
 };
 
 typedef std::map<MethodIdentifier, jmethodID> MethodIDMap;
-base::LazyInstance<MethodIDMap>::Leaky
-    g_method_id_map = LAZY_INSTANCE_INITIALIZER;
+
 const base::subtle::AtomicWord kUnlocked = 0;
 const base::subtle::AtomicWord kLocked = 1;
 base::subtle::AtomicWord g_method_id_map_lock = kUnlocked;
+JavaVM* g_jvm = NULL;
+// Leak the global app context, as it is used from a non-joinable worker thread
+// that may still be running at shutdown. There is no harm in doing this.
+base::LazyInstance<base::android::ScopedJavaGlobalRef<jobject> >::Leaky
+    g_application_context = LAZY_INSTANCE_INITIALIZER;
+base::LazyInstance<MethodIDMap> g_method_id_map = LAZY_INSTANCE_INITIALIZER;

[Ryan Sleevi, 2012/09/06 18:23:35]

BUG: This was Leaky, but when you moved it, you removed the Leaky

It seems like this should have the same lifetime as the g_application_context,
for the same reasons.

[Johnny(Jianning) Ding, 2012/09/07 04:33:07]

Done.

 
 std::string GetJavaExceptionInfo(JNIEnv* env, jthrowable java_throwable) {
   ScopedJavaLocalRef<jclass> throwable_clazz =
       GetClass(env, "java/lang/Throwable");
   jmethodID throwable_printstacktrace =
       GetMethodID(env, throwable_clazz, "printStackTrace",
                   "(Ljava/io/PrintStream;)V");
 
   // Create an instance of ByteArrayOutputStream.
   ScopedJavaLocalRef<jclass> bytearray_output_stream_clazz =
@@ skipping 83 matching lines @@
   }
   bool error = ClearException(env);
   DCHECK(!error);
   return true;
 }
 
 jmethodID GetMethodID(JNIEnv* env,
                       const JavaRef<jclass>& clazz,
                       const char* method_name,
                       const char* jni_signature) {
-  // We can't use clazz.env() as that may be from a different thread.
+  // Method clazz.env() can not be used as that may be from a different thread.

[Ryan Sleevi, 2012/09/06 18:23:35]

nit: s/Method //

[Johnny(Jianning) Ding, 2012/09/07 04:33:07]

Done.

   return GetMethodID(env, clazz.obj(), method_name, jni_signature);
 }
 
 jmethodID GetMethodID(JNIEnv* env,
                       jclass clazz,
                       const char* method_name,
                       const char* jni_signature) {
   jmethodID method_id =
       env->GetMethodID(clazz, method_name, jni_signature);
   CHECK(method_id && !ClearException(env)) << "Failed to find method " <<
@@ skipping 127 matching lines @@
   if (!HasException(env))
     return false;
   env->ExceptionDescribe();
   env->ExceptionClear();
   return true;
 }
 
 void CheckException(JNIEnv* env) {
   if (!HasException(env)) return;
 
-  // Ugh, we are going to die, might as well tell breakpad about it.
+  // Exception has been found, might as well tell breakpad about it.
   jthrowable java_throwable = env->ExceptionOccurred();
   if (!java_throwable) {
-    // Nothing we can do.
+    // Do nothing but return false.
     CHECK(false);
   }
 
-  // Clear the pending exception, we do have a reference to it.
+  // Clear the pending exception as it has been referred.

[Ryan Sleevi, 2012/09/06 18:23:35]

nit: Clear the pending exception, since a local reference is now held.

[Johnny(Jianning) Ding, 2012/09/07 04:33:07]

Done.

   env->ExceptionClear();
 
   // Set the exception_string in BuildInfo so that breakpad can read it.
   // RVO should avoid any extra copies of the exception string.
   base::android::BuildInfo::GetInstance()->set_java_exception_info(
       GetJavaExceptionInfo(env, java_throwable));
 
   // Now, feel good about it and die.
   CHECK(false);
 }
 
 }  // namespace android
 }  // namespace base