Add the condition variable to System.loadLibrary and implement UnregisterNatives.

Also change PrettyDescriptor now descriptors are String*s rather than StringPiece&s.

Change-Id: Id07affb26038f5f4a3bee4396c65f71d7bc38be3
diff --git a/src/jni_internal.cc b/src/jni_internal.cc
index 79986c2..c5b06bb 100644
--- a/src/jni_internal.cc
+++ b/src/jni_internal.cc
@@ -57,6 +57,7 @@
         jni_on_load_lock_(Mutex::Create("JNI_OnLoad lock")),
         jni_on_load_tid_(Thread::Current()->GetId()),
         jni_on_load_result_(kPending) {
+    pthread_cond_init(&jni_on_load_cond_, NULL);
   }
 
   ~SharedLibrary() {
@@ -81,8 +82,7 @@
       return true;
     }
 
-    UNIMPLEMENTED(ERROR) << "need to pthread_cond_wait!";
-    // MutexLock mu(jni_on_load_lock_);
+    MutexLock mu(jni_on_load_lock_);
     while (jni_on_load_result_ == kPending) {
       if (vm->verbose_jni) {
         LOG(INFO) << "[" << *self << " waiting for \"" << path_ << "\" "
@@ -90,7 +90,7 @@
       }
       Thread::State old_state = self->GetState();
       self->SetState(Thread::kWaiting); // TODO: VMWAIT
-      // pthread_cond_wait(&jni_on_load_cond_, &jni_on_load_lock_);
+      pthread_cond_wait(&jni_on_load_cond_, &(jni_on_load_lock_->lock_impl_));
       self->SetState(old_state);
     }
 
@@ -107,9 +107,8 @@
     jni_on_load_tid_ = 0;
 
     // Broadcast a wakeup to anybody sleeping on the condition variable.
-    UNIMPLEMENTED(ERROR) << "missing pthread_cond_broadcast";
-    // MutexLock mu(library->jni_on_load_lock_);
-    // pthread_cond_broadcast(&library->jni_on_load_cond_);
+    MutexLock mu(jni_on_load_lock_);
+    pthread_cond_broadcast(&jni_on_load_cond_);
   }
 
  private:
@@ -2021,11 +2020,13 @@
     SetPrimitiveArrayRegion<jshortArray, jshort, ShortArray>(ts, array, start, length, buf);
   }
 
-  static jint RegisterNatives(JNIEnv* env,
-      jclass clazz, const JNINativeMethod* methods, jint nMethods) {
+  static jint RegisterNatives(JNIEnv* env, jclass java_class, const JNINativeMethod* methods, jint method_count) {
     ScopedJniThreadState ts(env);
-    Class* klass = Decode<Class*>(ts, clazz);
-    for(int i = 0; i < nMethods; i++) {
+    Class* c = Decode<Class*>(ts, java_class);
+
+    JavaVMExt* vm = Runtime::Current()->GetJavaVM();
+
+    for (int i = 0; i < method_count; i++) {
       const char* name = methods[i].name;
       const char* sig = methods[i].signature;
 
@@ -2034,34 +2035,60 @@
         ++sig;
       }
 
-      Method* method = klass->FindDirectMethod(name, sig);
-      if (method == NULL) {
-        method = klass->FindVirtualMethod(name, sig);
+      Method* m = c->FindDirectMethod(name, sig);
+      if (m == NULL) {
+        m = c->FindVirtualMethod(name, sig);
       }
-      if (method == NULL) {
+      if (m == NULL) {
         Thread* self = Thread::Current();
-        std::string class_descriptor(klass->GetDescriptor()->ToModifiedUtf8());
+        std::string class_descriptor(c->GetDescriptor()->ToModifiedUtf8());
         self->ThrowNewException("Ljava/lang/NoSuchMethodError;",
             "no method \"%s.%s%s\"",
             class_descriptor.c_str(), name, sig);
         return JNI_ERR;
-      } else if (!method->IsNative()) {
+      } else if (!m->IsNative()) {
         Thread* self = Thread::Current();
-        std::string class_descriptor(klass->GetDescriptor()->ToModifiedUtf8());
+        std::string class_descriptor(c->GetDescriptor()->ToModifiedUtf8());
         self->ThrowNewException("Ljava/lang/NoSuchMethodError;",
             "method \"%s.%s%s\" is not native",
             class_descriptor.c_str(), name, sig);
         return JNI_ERR;
       }
-      method->RegisterNative(methods[i].fnPtr);
+
+      if (vm->verbose_jni) {
+        LOG(INFO) << "[Registering JNI native method "
+                  << PrettyMethod(m, true) << "]";
+      }
+
+      m->RegisterNative(methods[i].fnPtr);
     }
     return JNI_OK;
   }
 
-  static jint UnregisterNatives(JNIEnv* env, jclass clazz) {
+  static jint UnregisterNatives(JNIEnv* env, jclass java_class) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
-    return 0;
+    Class* c = Decode<Class*>(ts, java_class);
+
+    JavaVMExt* vm = Runtime::Current()->GetJavaVM();
+    if (vm->verbose_jni) {
+      LOG(INFO) << "[Unregistering JNI native methods for "
+                << PrettyDescriptor(c->GetDescriptor()) << "]";
+    }
+
+    for (size_t i = 0; i < c->NumDirectMethods(); ++i) {
+      Method* m = c->GetDirectMethod(i);
+      if (m->IsNative()) {
+        m->UnregisterNative();
+      }
+    }
+    for (size_t i = 0; i < c->NumVirtualMethods(); ++i) {
+      Method* m = c->GetVirtualMethod(i);
+      if (m->IsNative()) {
+        m->UnregisterNative();
+      }
+    }
+
+    return JNI_OK;
   }
 
   static jint MonitorEnter(JNIEnv* env, jobject obj) {
@@ -2630,8 +2657,6 @@
 
   // Create a new entry.
   library = new SharedLibrary(path, handle, class_loader);
-  UNIMPLEMENTED(ERROR) << "missing pthread_cond_init";
-  // pthread_cond_init(&library->onLoadCond, NULL);
 
   libraries[path] = library;
 
diff --git a/src/jni_internal_test.cc b/src/jni_internal_test.cc
index 53b8334..81fda42 100644
--- a/src/jni_internal_test.cc
+++ b/src/jni_internal_test.cc
@@ -14,6 +14,10 @@
  protected:
   virtual void SetUp() {
     CommonTest::SetUp();
+
+    // Turn on -verbose:jni for the JNI tests.
+    Runtime::Current()->GetJavaVM()->verbose_jni = true;
+
     env_ = Thread::Current()->GetJniEnv();
     aioobe_ = env_->FindClass("java/lang/ArrayIndexOutOfBoundsException");
     CHECK(aioobe_ != NULL);
@@ -233,6 +237,10 @@
   ASSERT_TRUE(mid2 != NULL);
 }
 
+void BogusMethod() {
+  // You can't pass NULL function pointers to RegisterNatives.
+}
+
 TEST_F(JniInternalTest, RegisterNatives) {
   jclass jlobject = env_->FindClass("java/lang/Object");
   jclass jlnsme = env_->FindClass("java/lang/NoSuchMethodError");
@@ -257,10 +265,12 @@
 
   // Check that registering native methods is successful
   {
-    JNINativeMethod methods[] = {{"hashCode", "()I", NULL}};
+    JNINativeMethod methods[] = {{"hashCode", "()I", reinterpret_cast<void*>(BogusMethod)}};
     env_->RegisterNatives(jlobject, methods, 1);
   }
   EXPECT_FALSE(env_->ExceptionCheck());
+
+  env_->UnregisterNatives(jlobject);
 }
 
 #define EXPECT_PRIMITIVE_ARRAY(new_fn, get_region_fn, set_region_fn, scalar_type, expected_class_descriptor) \
diff --git a/src/object.h b/src/object.h
index 7004e59..3c7ec49 100644
--- a/src/object.h
+++ b/src/object.h
@@ -635,9 +635,14 @@
   }
 
   void RegisterNative(const void* native_method) {
+    CHECK(native_method != NULL);
     native_method_ = native_method;
   }
 
+  void UnregisterNative() {
+    native_method_ = NULL;
+  }
+
   static MemberOffset NativeMethodOffset() {
     return MemberOffset(OFFSETOF_MEMBER(Method, native_method_));
   }
diff --git a/src/runtime.cc b/src/runtime.cc
index 476297b..b009e22 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -193,12 +193,11 @@
   const char* boot_class_path = getenv("BOOTCLASSPATH");
   parsed->boot_image_ = NULL;
 #ifdef NDEBUG
-  // -Xcheck:jni and -verbose:jni are off by default for regular builds...
+  // -Xcheck:jni is off by default for regular builds...
   parsed->check_jni_ = false;
 #else
   // ...but on by default in debug builds.
   parsed->check_jni_ = true;
-  parsed->verbose_.insert("jni");
 #endif
   parsed->heap_initial_size_ = Heap::kInitialSize;
   parsed->heap_maximum_size_ = Heap::kMaximumSize;
diff --git a/src/thread.h b/src/thread.h
index 979a3e3..7204577 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -55,6 +55,8 @@
 
   pthread_mutex_t lock_impl_;
 
+  friend class SharedLibrary; // For lock_impl_.
+
   DISALLOW_COPY_AND_ASSIGN(Mutex);
 };
 
diff --git a/src/utils.cc b/src/utils.cc
index f8a55ce..c24504e 100644
--- a/src/utils.cc
+++ b/src/utils.cc
@@ -25,9 +25,11 @@
   return contents;
 }
 
-std::string PrettyDescriptor(const StringPiece& descriptor) {
+std::string PrettyDescriptor(const String* java_descriptor) {
+  std::string descriptor(java_descriptor->ToModifiedUtf8());
+
   // Count the number of '['s to get the dimensionality.
-  const char* c = descriptor.data();
+  const char* c = descriptor.c_str();
   size_t dim = 0;
   while (*c == '[') {
     dim++;
@@ -51,7 +53,7 @@
     case 'J': c = "long;"; break;
     case 'S': c = "short;"; break;
     case 'Z': c = "boolean;"; break;
-    default: return descriptor.ToString();
+    default: return descriptor;
     }
   }
 
@@ -78,7 +80,7 @@
     return "null";
   }
   Class* c = m->GetDeclaringClass();
-  std::string result(PrettyDescriptor(c->GetDescriptor()->ToModifiedUtf8()));
+  std::string result(PrettyDescriptor(c->GetDescriptor()));
   result += '.';
   result += m->GetName()->ToModifiedUtf8();
   if (with_signature) {
@@ -96,9 +98,9 @@
   if (obj->GetClass() == NULL) {
     return "(raw)";
   }
-  std::string result(PrettyDescriptor(obj->GetClass()->GetDescriptor()->ToModifiedUtf8()));
+  std::string result(PrettyDescriptor(obj->GetClass()->GetDescriptor()));
   if (obj->IsClass()) {
-    result += "<" + PrettyDescriptor(obj->AsClass()->GetDescriptor()->ToModifiedUtf8()) + ">";
+    result += "<" + PrettyDescriptor(obj->AsClass()->GetDescriptor()) + ">";
   }
   return result;
 }
diff --git a/src/utils.h b/src/utils.h
index 5c2a44a..4283f47 100644
--- a/src/utils.h
+++ b/src/utils.h
@@ -12,6 +12,7 @@
 
 class Method;
 class Object;
+class String;
 
 template<typename T>
 static inline bool IsPowerOfTwo(T x) {
@@ -139,7 +140,7 @@
 // Returns a human-readable equivalent of 'descriptor'. So "I" would be "int",
 // "[[I" would be "int[][]", "[Ljava/lang/String;" would be
 // "java.lang.String[]", and so forth.
-std::string PrettyDescriptor(const StringPiece& descriptor);
+std::string PrettyDescriptor(const String* descriptor);
 
 // Returns a human-readable signature for 'm'. Something like "a.b.C.m" or
 // "a.b.C.m(II)V" (depending on the value of 'with_signature').
diff --git a/src/utils_test.cc b/src/utils_test.cc
index 8f2b3ba..fa5d798 100644
--- a/src/utils_test.cc
+++ b/src/utils_test.cc
@@ -11,44 +11,51 @@
 class UtilsTest : public CommonTest {
 };
 
-TEST(PrettyDescriptorTest, ArrayReferences) {
-  EXPECT_EQ("java.lang.Class[]", PrettyDescriptor("[Ljava/lang/Class;"));
-  EXPECT_EQ("java.lang.Class[][]", PrettyDescriptor("[[Ljava/lang/Class;"));
+#define EXPECT_DESCRIPTOR(pretty_descriptor, descriptor) \
+  do { \
+    String* s = String::AllocFromModifiedUtf8(descriptor); \
+    std::string result(PrettyDescriptor(s)); \
+    EXPECT_EQ(pretty_descriptor, result); \
+  } while (false)
+
+TEST_F(UtilsTest, PrettyDescriptor_ArrayReferences) {
+  EXPECT_DESCRIPTOR("java.lang.Class[]", "[Ljava/lang/Class;");
+  EXPECT_DESCRIPTOR("java.lang.Class[][]", "[[Ljava/lang/Class;");
 }
 
-TEST(PrettyDescriptorTest, ScalarReferences) {
-  EXPECT_EQ("java.lang.String", PrettyDescriptor("Ljava.lang.String;"));
-  EXPECT_EQ("java.lang.String", PrettyDescriptor("Ljava/lang/String;"));
+TEST_F(UtilsTest, PrettyDescriptor_ScalarReferences) {
+  EXPECT_DESCRIPTOR("java.lang.String", "Ljava.lang.String;");
+  EXPECT_DESCRIPTOR("java.lang.String", "Ljava/lang/String;");
 }
 
-TEST(PrettyDescriptorTest, PrimitiveArrays) {
-  EXPECT_EQ("boolean[]", PrettyDescriptor("[Z"));
-  EXPECT_EQ("boolean[][]", PrettyDescriptor("[[Z"));
-  EXPECT_EQ("byte[]", PrettyDescriptor("[B"));
-  EXPECT_EQ("byte[][]", PrettyDescriptor("[[B"));
-  EXPECT_EQ("char[]", PrettyDescriptor("[C"));
-  EXPECT_EQ("char[][]", PrettyDescriptor("[[C"));
-  EXPECT_EQ("double[]", PrettyDescriptor("[D"));
-  EXPECT_EQ("double[][]", PrettyDescriptor("[[D"));
-  EXPECT_EQ("float[]", PrettyDescriptor("[F"));
-  EXPECT_EQ("float[][]", PrettyDescriptor("[[F"));
-  EXPECT_EQ("int[]", PrettyDescriptor("[I"));
-  EXPECT_EQ("int[][]", PrettyDescriptor("[[I"));
-  EXPECT_EQ("long[]", PrettyDescriptor("[J"));
-  EXPECT_EQ("long[][]", PrettyDescriptor("[[J"));
-  EXPECT_EQ("short[]", PrettyDescriptor("[S"));
-  EXPECT_EQ("short[][]", PrettyDescriptor("[[S"));
+TEST_F(UtilsTest, PrettyDescriptor_PrimitiveArrays) {
+  EXPECT_DESCRIPTOR("boolean[]", "[Z");
+  EXPECT_DESCRIPTOR("boolean[][]", "[[Z");
+  EXPECT_DESCRIPTOR("byte[]", "[B");
+  EXPECT_DESCRIPTOR("byte[][]", "[[B");
+  EXPECT_DESCRIPTOR("char[]", "[C");
+  EXPECT_DESCRIPTOR("char[][]", "[[C");
+  EXPECT_DESCRIPTOR("double[]", "[D");
+  EXPECT_DESCRIPTOR("double[][]", "[[D");
+  EXPECT_DESCRIPTOR("float[]", "[F");
+  EXPECT_DESCRIPTOR("float[][]", "[[F");
+  EXPECT_DESCRIPTOR("int[]", "[I");
+  EXPECT_DESCRIPTOR("int[][]", "[[I");
+  EXPECT_DESCRIPTOR("long[]", "[J");
+  EXPECT_DESCRIPTOR("long[][]", "[[J");
+  EXPECT_DESCRIPTOR("short[]", "[S");
+  EXPECT_DESCRIPTOR("short[][]", "[[S");
 }
 
-TEST(PrettyDescriptorTest, PrimitiveScalars) {
-  EXPECT_EQ("boolean", PrettyDescriptor("Z"));
-  EXPECT_EQ("byte", PrettyDescriptor("B"));
-  EXPECT_EQ("char", PrettyDescriptor("C"));
-  EXPECT_EQ("double", PrettyDescriptor("D"));
-  EXPECT_EQ("float", PrettyDescriptor("F"));
-  EXPECT_EQ("int", PrettyDescriptor("I"));
-  EXPECT_EQ("long", PrettyDescriptor("J"));
-  EXPECT_EQ("short", PrettyDescriptor("S"));
+TEST_F(UtilsTest, PrettyDescriptor_PrimitiveScalars) {
+  EXPECT_DESCRIPTOR("boolean", "Z");
+  EXPECT_DESCRIPTOR("byte", "B");
+  EXPECT_DESCRIPTOR("char", "C");
+  EXPECT_DESCRIPTOR("double", "D");
+  EXPECT_DESCRIPTOR("float", "F");
+  EXPECT_DESCRIPTOR("int", "I");
+  EXPECT_DESCRIPTOR("long", "J");
+  EXPECT_DESCRIPTOR("short", "S");
 }
 
 TEST_F(UtilsTest, PrettyType) {