Allow OpenSSLPrivateKeyStore and Android global app context leaking.

base/android/jni_android.cc

Index: base/android/jni_android.cc
===================================================================
--- base/android/jni_android.cc	(revision 154951)
+++ base/android/jni_android.cc	(working copy)
@@ -18,11 +18,6 @@
 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;
@@ -48,11 +43,16 @@
 };
 
 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 =
@@ -156,7 +156,7 @@
                       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);
 }
 
@@ -304,14 +304,14 @@
 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.