Finish off the new JNI implementation.

There are a handful of remaining TODOs, but this gives us complete coverage.

Change-Id: Ibee38e6a87a0fcfae769d991125b0551243c8eeb

9 files changed, 299 insertions, 207 deletions

diff --git a/src/indirect_reference_table.cc b/src/indirect_reference_table.cc
index 2ea243490b..76d53790a4 100644
--- a/src/indirect_reference_table.cc
+++ b/src/indirect_reference_table.cc
@@ -318,7 +318,7 @@ std::ostream& operator<<(std::ostream& os, IndirectRefKind rhs) {
 
 void IndirectReferenceTable::Dump() const {
   LOG(WARNING) << kind_ << " table dump:";
-  std::vector<Object*> entries(table_, table_ + Capacity());
+  std::vector<const Object*> entries(table_, table_ + Capacity());
   ReferenceTable::Dump(entries);
 }
 
diff --git a/src/jni_internal.cc b/src/jni_internal.cc
index d11f2a7acb..a95ff81f22 100644
--- a/src/jni_internal.cc
+++ b/src/jni_internal.cc
@@ -165,6 +165,10 @@ class ScopedJniThreadState {
     return self_;
   }
 
+  JavaVMExt* Vm() {
+    return env_->vm;
+  }
+
  private:
   static Thread* ThreadForEnv(JNIEnv* env) {
     // TODO: need replacement for gDvmJni.
@@ -240,7 +244,7 @@ jweak AddWeakGlobalReference(ScopedJniThreadState& ts, Object* obj) {
   if (obj == NULL) {
     return NULL;
   }
-  JavaVMExt* vm = Runtime::Current()->GetJavaVM();
+  JavaVMExt* vm = ts.Vm();
   IndirectReferenceTable& weak_globals = vm->weak_globals;
   MutexLock mu(vm->weak_globals_lock);
   IndirectRef ref = weak_globals.Add(IRT_FIRST_SEGMENT, obj);
@@ -384,7 +388,7 @@ JValue InvokeWithVarArgs(ScopedJniThreadState& ts, jobject obj,
   return InvokeWithArgArray(ts, receiver, method, arg_array.get());
 }
 
-static Method* FindVirtualMethod(Object* receiver, Method* method) {
+Method* FindVirtualMethod(Object* receiver, Method* method) {
   return receiver->GetClass()->GetMethodByVtableIndex(method->GetVtableIndex());
 }
 
@@ -490,6 +494,18 @@ jfieldID FindFieldID(ScopedJniThreadState& ts, jclass jni_class, const char* nam
   return reinterpret_cast<jfieldID>(fid);
 }
 
+void PinPrimitiveArray(ScopedJniThreadState& ts, const Array* array) {
+  JavaVMExt* vm = ts.Vm();
+  MutexLock mu(vm->pins_lock);
+  vm->pin_table.Add(array);
+}
+
+void UnpinPrimitiveArray(ScopedJniThreadState& ts, const Array* array) {
+  JavaVMExt* vm = ts.Vm();
+  MutexLock mu(vm->pins_lock);
+  vm->pin_table.Remove(array);
+}
+
 template<typename JniT, typename ArtT>
 JniT NewPrimitiveArray(ScopedJniThreadState& ts, jsize length) {
   CHECK_GE(length, 0); // TODO: ReportJniError
@@ -497,6 +513,24 @@ JniT NewPrimitiveArray(ScopedJniThreadState& ts, jsize length) {
   return AddLocalReference<JniT>(ts, result);
 }
 
+template <typename ArrayT, typename CArrayT, typename ArtArrayT>
+CArrayT GetPrimitiveArray(ScopedJniThreadState& ts, ArrayT java_array, jboolean* is_copy) {
+  ArtArrayT* array = Decode<ArtArrayT*>(ts, java_array);
+  PinPrimitiveArray(ts, array);
+  if (is_copy != NULL) {
+    *is_copy = JNI_FALSE;
+  }
+  return array->GetData();
+}
+
+template <typename ArrayT>
+void ReleasePrimitiveArray(ScopedJniThreadState& ts, ArrayT java_array, jint mode) {
+  if (mode != JNI_COMMIT) {
+    Array* array = Decode<Array*>(ts, java_array);
+    UnpinPrimitiveArray(ts, array);
+  }
+}
+
 void ThrowAIOOBE(ScopedJniThreadState& ts, Array* array, jsize start, jsize length, const char* identifier) {
   std::string type(PrettyType(array));
   ts.Self()->ThrowNewException("Ljava/lang/ArrayIndexOutOfBoundsException;",
@@ -509,7 +543,7 @@ void ThrowSIOOBE(ScopedJniThreadState& ts, jsize start, jsize length, jsize arra
 }
 
 template <typename JavaArrayT, typename JavaT, typename ArrayT>
-static void GetPrimitiveArrayRegion(ScopedJniThreadState& ts, JavaArrayT java_array, jsize start, jsize length, JavaT* buf) {
+void GetPrimitiveArrayRegion(ScopedJniThreadState& ts, JavaArrayT java_array, jsize start, jsize length, JavaT* buf) {
   ArrayT* array = Decode<ArrayT*>(ts, java_array);
   if (start < 0 || length < 0 || start + length > array->GetLength()) {
     ThrowAIOOBE(ts, array, start, length, "src");
@@ -520,7 +554,7 @@ static void GetPrimitiveArrayRegion(ScopedJniThreadState& ts, JavaArrayT java_ar
 }
 
 template <typename JavaArrayT, typename JavaT, typename ArrayT>
-static void SetPrimitiveArrayRegion(ScopedJniThreadState& ts, JavaArrayT java_array, jsize start, jsize length, const JavaT* buf) {
+void SetPrimitiveArrayRegion(ScopedJniThreadState& ts, JavaArrayT java_array, jsize start, jsize length, const JavaT* buf) {
   ArrayT* array = Decode<ArrayT*>(ts, java_array);
   if (start < 0 || length < 0 || start + length > array->GetLength()) {
     ThrowAIOOBE(ts, array, start, length, "dst");
@@ -530,17 +564,45 @@ static void SetPrimitiveArrayRegion(ScopedJniThreadState& ts, JavaArrayT java_ar
   }
 }
 
-static jclass InitDirectByteBufferClass(JNIEnv* env) {
+jclass InitDirectByteBufferClass(JNIEnv* env) {
   ScopedLocalRef<jclass> buffer_class(env, env->FindClass("java/nio/ReadWriteDirectByteBuffer"));
   CHECK(buffer_class.get() != NULL);
   return reinterpret_cast<jclass>(env->NewGlobalRef(buffer_class.get()));
 }
 
-static jclass GetDirectByteBufferClass(JNIEnv* env) {
+jclass GetDirectByteBufferClass(JNIEnv* env) {
   static jclass buffer_class = InitDirectByteBufferClass(env);
   return buffer_class;
 }
 
+jint JII_AttachCurrentThread(JavaVM* vm, JNIEnv** p_env, void* thr_args, bool as_daemon) {
+  if (vm == NULL || p_env == NULL) {
+    return JNI_ERR;
+  }
+
+  JavaVMAttachArgs* in_args = static_cast<JavaVMAttachArgs*>(thr_args);
+  JavaVMAttachArgs args;
+  if (thr_args == NULL) {
+    // Allow the v1.1 calling convention.
+    args.version = JNI_VERSION_1_2;
+    args.name = NULL;
+    args.group = NULL; // TODO: get "main" thread group
+  } else {
+    args.version = in_args->version;
+    args.name = in_args->name;
+    if (in_args->group != NULL) {
+      UNIMPLEMENTED(WARNING) << "thr_args->group != NULL";
+      args.group = NULL; // TODO: decode in_args->group
+    } else {
+      args.group = NULL; // TODO: get "main" thread group
+    }
+  }
+  CHECK_GE(args.version, JNI_VERSION_1_2);
+
+  Runtime* runtime = reinterpret_cast<JavaVMExt*>(vm)->runtime;
+  return runtime->AttachCurrentThread(args.name, p_env, as_daemon) ? JNI_OK : JNI_ERR;
+}
+
 }  // namespace
 
 class JNI {
@@ -612,7 +674,7 @@ class JNI {
 
   static jboolean IsInstanceOf(JNIEnv* env, jobject jobj, jclass clazz) {
     ScopedJniThreadState ts(env);
-    CHECK_NE(static_cast<jclass>(NULL), clazz);
+    CHECK_NE(static_cast<jclass>(NULL), clazz); // TODO: ReportJniError
     if (jobj == NULL) {
       // NB. JNI is different from regular Java instanceof in this respect
       return JNI_TRUE;
@@ -744,7 +806,7 @@ class JNI {
       return NULL;
     }
 
-    JavaVMExt* vm = Runtime::Current()->GetJavaVM();
+    JavaVMExt* vm = ts.Vm();
     IndirectReferenceTable& globals = vm->globals;
     MutexLock mu(vm->globals_lock);
     IndirectRef ref = globals.Add(IRT_FIRST_SEGMENT, Decode<Object*>(ts, obj));
@@ -757,7 +819,7 @@ class JNI {
       return;
     }
 
-    JavaVMExt* vm = Runtime::Current()->GetJavaVM();
+    JavaVMExt* vm = ts.Vm();
     IndirectReferenceTable& globals = vm->globals;
     MutexLock mu(vm->globals_lock);
 
@@ -778,7 +840,7 @@ class JNI {
       return;
     }
 
-    JavaVMExt* vm = Runtime::Current()->GetJavaVM();
+    JavaVMExt* vm = ts.Vm();
     IndirectReferenceTable& weak_globals = vm->weak_globals;
     MutexLock mu(vm->weak_globals_lock);
 
@@ -1737,37 +1799,56 @@ class JNI {
     }
   }
 
-  static const jchar* GetStringChars(JNIEnv* env, jstring str, jboolean* isCopy) {
+  static const jchar* GetStringChars(JNIEnv* env, jstring java_string, jboolean* is_copy) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
-    return NULL;
+    String* s = Decode<String*>(ts, java_string);
+    const CharArray* chars = s->GetCharArray();
+    PinPrimitiveArray(ts, chars);
+    if (is_copy != NULL) {
+      *is_copy = JNI_FALSE;
+    }
+    return chars->GetData() + s->GetOffset();
   }
 
-  static void ReleaseStringChars(JNIEnv* env, jstring str, const jchar* chars) {
+  static void ReleaseStringChars(JNIEnv* env, jstring java_string, const jchar* chars) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
+    UnpinPrimitiveArray(ts, Decode<String*>(ts, java_string)->GetCharArray());
   }
 
-  static const char* GetStringUTFChars(JNIEnv* env, jstring str, jboolean* isCopy) {
+  static const jchar* GetStringCritical(JNIEnv* env, jstring java_string, jboolean* is_copy) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
-    return NULL;
+    return GetStringChars(env, java_string, is_copy);
   }
 
-  static void ReleaseStringUTFChars(JNIEnv* env, jstring str, const char* chars) {
+  static void ReleaseStringCritical(JNIEnv* env, jstring java_string, const jchar* chars) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
+    return ReleaseStringChars(env, java_string, chars);
   }
 
-  static const jchar* GetStringCritical(JNIEnv* env, jstring s, jboolean* isCopy) {
+  static const char* GetStringUTFChars(JNIEnv* env, jstring java_string, jboolean* is_copy) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
-    return NULL;
+    if (java_string == NULL) {
+      return NULL;
+    }
+    if (is_copy != NULL) {
+      *is_copy = JNI_TRUE;
+    }
+    String* s = Decode<String*>(ts, java_string);
+    size_t byte_count = s->GetUtfLength();
+    char* bytes = new char[byte_count + 1];
+    if (bytes == NULL) {
+      UNIMPLEMENTED(WARNING) << "need to Throw OOME";
+      return NULL;
+    }
+    const uint16_t* chars = s->GetCharArray()->GetData() + s->GetOffset();
+    ConvertUtf16ToModifiedUtf8(bytes, chars, s->GetLength());
+    bytes[byte_count] = '\0';
+    return bytes;
   }
 
-  static void ReleaseStringCritical(JNIEnv* env, jstring s, const jchar* cstr) {
+  static void ReleaseStringUTFChars(JNIEnv* env, jstring, const char* chars) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
+    delete[] chars;
   }
 
   static jsize GetArrayLength(JNIEnv* env, jarray java_array) {
@@ -1838,15 +1919,20 @@ class JNI {
     descriptor += element_class->GetDescriptor()->ToModifiedUtf8();
 
     // Find the class.
-    ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
-    // TODO: need to get the appropriate ClassLoader.
-    Class* array_class = class_linker->FindClass(descriptor, NULL);
-    if (array_class == NULL) {
+    ScopedLocalRef<jclass> java_array_class(env, FindClass(env, descriptor.c_str()));
+    if (java_array_class.get() == NULL) {
       return NULL;
     }
 
+    // Allocate and initialize if necessary.
+    Class* array_class = Decode<Class*>(ts, java_array_class.get());
     ObjectArray<Object>* result = ObjectArray<Object>::Alloc(array_class, length);
-    CHECK(initial_element == NULL);  // TODO: support initial_element
+    if (initial_element != NULL) {
+      Object* initial_object = Decode<Object*>(ts, initial_element);
+      for (jsize i = 0; i < length; ++i) {
+        result->Set(i, initial_object);
+      }
+    }
     return AddLocalReference<jobjectArray>(ts, result);
   }
 
@@ -1855,115 +1941,94 @@ class JNI {
     return NewPrimitiveArray<jshortArray, ShortArray>(ts, length);
   }
 
-  static void* GetPrimitiveArrayCritical(JNIEnv* env, jarray array, jboolean* isCopy) {
+  static void* GetPrimitiveArrayCritical(JNIEnv* env, jarray array, jboolean* is_copy) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
-    return NULL;
+    return GetPrimitiveArray<jarray, jbyte*, ByteArray>(ts, array, is_copy);
   }
 
-  static void ReleasePrimitiveArrayCritical(JNIEnv* env, jarray array, void* carray, jint mode) {
+  static void ReleasePrimitiveArrayCritical(JNIEnv* env, jarray array, void* data, jint mode) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
+    ReleasePrimitiveArray(ts, array, mode);
   }
 
-  static jboolean* GetBooleanArrayElements(JNIEnv* env,
-      jbooleanArray array, jboolean* isCopy) {
+  static jboolean* GetBooleanArrayElements(JNIEnv* env, jbooleanArray array, jboolean* is_copy) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
-    return NULL;
+    return GetPrimitiveArray<jbooleanArray, jboolean*, BooleanArray>(ts, array, is_copy);
   }
 
-  static jbyte* GetByteArrayElements(JNIEnv* env, jbyteArray array, jboolean* isCopy) {
+  static jbyte* GetByteArrayElements(JNIEnv* env, jbyteArray array, jboolean* is_copy) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
-    return NULL;
+    return GetPrimitiveArray<jbyteArray, jbyte*, ByteArray>(ts, array, is_copy);
   }
 
-  static jchar* GetCharArrayElements(JNIEnv* env, jcharArray array, jboolean* isCopy) {
+  static jchar* GetCharArrayElements(JNIEnv* env, jcharArray array, jboolean* is_copy) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
-    return NULL;
+    return GetPrimitiveArray<jcharArray, jchar*, CharArray>(ts, array, is_copy);
   }
 
-  static jshort* GetShortArrayElements(JNIEnv* env,
-      jshortArray array, jboolean* isCopy) {
+  static jdouble* GetDoubleArrayElements(JNIEnv* env, jdoubleArray array, jboolean* is_copy) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
-    return NULL;
+    return GetPrimitiveArray<jdoubleArray, jdouble*, DoubleArray>(ts, array, is_copy);
   }
 
-  static jint* GetIntArrayElements(JNIEnv* env, jintArray array, jboolean* isCopy) {
+  static jfloat* GetFloatArrayElements(JNIEnv* env, jfloatArray array, jboolean* is_copy) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
-    return NULL;
+    return GetPrimitiveArray<jfloatArray, jfloat*, FloatArray>(ts, array, is_copy);
   }
 
-  static jlong* GetLongArrayElements(JNIEnv* env, jlongArray array, jboolean* isCopy) {
+  static jint* GetIntArrayElements(JNIEnv* env, jintArray array, jboolean* is_copy) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
-    return NULL;
+    return GetPrimitiveArray<jintArray, jint*, IntArray>(ts, array, is_copy);
   }
 
-  static jfloat* GetFloatArrayElements(JNIEnv* env,
-      jfloatArray array, jboolean* isCopy) {
+  static jlong* GetLongArrayElements(JNIEnv* env, jlongArray array, jboolean* is_copy) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
-    return NULL;
+    return GetPrimitiveArray<jlongArray, jlong*, LongArray>(ts, array, is_copy);
   }
 
-  static jdouble* GetDoubleArrayElements(JNIEnv* env,
-      jdoubleArray array, jboolean* isCopy) {
+  static jshort* GetShortArrayElements(JNIEnv* env, jshortArray array, jboolean* is_copy) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
-    return NULL;
+    return GetPrimitiveArray<jshortArray, jshort*, ShortArray>(ts, array, is_copy);
   }
 
-  static void ReleaseBooleanArrayElements(JNIEnv* env,
-      jbooleanArray array, jboolean* elems, jint mode) {
+  static void ReleaseBooleanArrayElements(JNIEnv* env, jbooleanArray array, jboolean* data, jint mode) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
+    ReleasePrimitiveArray(ts, array, mode);
   }
 
-  static void ReleaseByteArrayElements(JNIEnv* env,
-      jbyteArray array, jbyte* elems, jint mode) {
+  static void ReleaseByteArrayElements(JNIEnv* env, jbyteArray array, jbyte* data, jint mode) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
+    ReleasePrimitiveArray(ts, array, mode);
   }
 
-  static void ReleaseCharArrayElements(JNIEnv* env,
-      jcharArray array, jchar* elems, jint mode) {
+  static void ReleaseCharArrayElements(JNIEnv* env, jcharArray array, jchar* data, jint mode) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
+    ReleasePrimitiveArray(ts, array, mode);
   }
 
-  static void ReleaseShortArrayElements(JNIEnv* env,
-      jshortArray array, jshort* elems, jint mode) {
+  static void ReleaseDoubleArrayElements(JNIEnv* env, jdoubleArray array, jdouble* data, jint mode) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
+    ReleasePrimitiveArray(ts, array, mode);
   }
 
-  static void ReleaseIntArrayElements(JNIEnv* env,
-      jintArray array, jint* elems, jint mode) {
+  static void ReleaseFloatArrayElements(JNIEnv* env, jfloatArray array, jfloat* data, jint mode) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
+    ReleasePrimitiveArray(ts, array, mode);
   }
 
-  static void ReleaseLongArrayElements(JNIEnv* env,
-      jlongArray array, jlong* elems, jint mode) {
+  static void ReleaseIntArrayElements(JNIEnv* env, jintArray array, jint* data, jint mode) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
+    ReleasePrimitiveArray(ts, array, mode);
   }
 
-  static void ReleaseFloatArrayElements(JNIEnv* env,
-      jfloatArray array, jfloat* elems, jint mode) {
+  static void ReleaseLongArrayElements(JNIEnv* env, jlongArray array, jlong* data, jint mode) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
+    ReleasePrimitiveArray(ts, array, mode);
   }
 
-  static void ReleaseDoubleArrayElements(JNIEnv* env,
-      jdoubleArray array, jdouble* elems, jint mode) {
+  static void ReleaseShortArrayElements(JNIEnv* env, jshortArray array, jshort* data, jint mode) {
     ScopedJniThreadState ts(env);
-    UNIMPLEMENTED(FATAL);
+    ReleasePrimitiveArray(ts, array, mode);
   }
 
   static void GetBooleanArrayRegion(JNIEnv* env, jbooleanArray array, jsize start, jsize length, jboolean* buf) {
@@ -2050,8 +2115,6 @@ class JNI {
     ScopedJniThreadState ts(env);
     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;
@@ -2081,7 +2144,7 @@ class JNI {
         return JNI_ERR;
       }
 
-      if (vm->verbose_jni) {
+      if (ts.Vm()->verbose_jni) {
         LOG(INFO) << "[Registering JNI native method "
                   << PrettyMethod(m, true) << "]";
       }
@@ -2095,8 +2158,7 @@ class JNI {
     ScopedJniThreadState ts(env);
     Class* c = Decode<Class*>(ts, java_class);
 
-    JavaVMExt* vm = Runtime::Current()->GetJavaVM();
-    if (vm->verbose_jni) {
+    if (ts.Vm()->verbose_jni) {
       LOG(INFO) << "[Unregistering JNI native methods for "
                 << PrettyDescriptor(c->GetDescriptor()) << "]";
     }
@@ -2144,8 +2206,8 @@ class JNI {
     ScopedJniThreadState ts(env);
 
     // The address may not be NULL, and the capacity must be > 0.
-    CHECK(address != NULL);
-    CHECK_GT(capacity, 0);
+    CHECK(address != NULL); // TODO: ReportJniError
+    CHECK_GT(capacity, 0); // TODO: ReportJniError
 
     jclass buffer_class = GetDirectByteBufferClass(env);
     jmethodID mid = env->GetMethodID(buffer_class, "<init>", "(II)V");
@@ -2178,7 +2240,7 @@ class JNI {
   static jobjectRefType GetObjectRefType(JNIEnv* env, jobject java_object) {
     ScopedJniThreadState ts(env);
 
-    CHECK(java_object != NULL);
+    CHECK(java_object != NULL); // TODO: ReportJniError
 
     // Do we definitely know what kind of reference this is?
     IndirectRef ref = reinterpret_cast<IndirectRef>(java_object);
@@ -2452,9 +2514,10 @@ static const size_t kMonitorsMax = 4096; // Arbitrary sanity check.
 static const size_t kLocalsInitial = 64; // Arbitrary.
 static const size_t kLocalsMax = 512; // Arbitrary sanity check.
 
-JNIEnvExt::JNIEnvExt(Thread* self, bool check_jni)
+JNIEnvExt::JNIEnvExt(Thread* self, JavaVMExt* vm)
     : self(self),
-      check_jni(check_jni),
+      vm(vm),
+      check_jni(vm->check_jni),
       critical(false),
       monitors("monitors", kMonitorsInitial, kMonitorsMax),
       locals(kLocalsInitial, kLocalsMax, kLocal) {
@@ -2514,43 +2577,11 @@ class JII {
   }
 
   static jint AttachCurrentThread(JavaVM* vm, JNIEnv** p_env, void* thr_args) {
-    if (vm == NULL || p_env == NULL) {
-      return JNI_ERR;
-    }
-    JavaVMExt* raw_vm = reinterpret_cast<JavaVMExt*>(vm);
-    Runtime* runtime = raw_vm->runtime;
-    const char* name = NULL;
-    if (thr_args != NULL) {
-      // TODO: check version
-      name = static_cast<JavaVMAttachArgs*>(thr_args)->name;
-      // TODO: thread group
-    }
-    bool success = runtime->AttachCurrentThread(name, p_env);
-    if (!success) {
-      return JNI_ERR;
-    } else {
-      return JNI_OK;
-    }
+    return JII_AttachCurrentThread(vm, p_env, thr_args, false);
   }
 
   static jint AttachCurrentThreadAsDaemon(JavaVM* vm, JNIEnv** p_env, void* thr_args) {
-    if (vm == NULL || p_env == NULL) {
-      return JNI_ERR;
-    }
-    JavaVMExt* raw_vm = reinterpret_cast<JavaVMExt*>(vm);
-    Runtime* runtime = raw_vm->runtime;
-    const char* name = NULL;
-    if (thr_args != NULL) {
-      // TODO: check version
-      name = static_cast<JavaVMAttachArgs*>(thr_args)->name;
-      // TODO: thread group
-    }
-    bool success = runtime->AttachCurrentThreadAsDaemon(name, p_env);
-    if (!success) {
-      return JNI_ERR;
-    } else {
-      return JNI_OK;
-    }
+    return JII_AttachCurrentThread(vm, p_env, thr_args, true);
   }
 
   static jint DetachCurrentThread(JavaVM* vm) {
@@ -2605,6 +2636,7 @@ JavaVMExt::JavaVMExt(Runtime* runtime, bool check_jni, bool verbose_jni)
     : runtime(runtime),
       check_jni(check_jni),
       verbose_jni(verbose_jni),
+      pins_lock(Mutex::Create("JNI pin table lock")),
       pin_table("pin table", kPinTableInitialSize, kPinTableMaxSize),
       globals_lock(Mutex::Create("JNI global reference table lock")),
       globals(kGlobalsInitial, kGlobalsMax, kGlobal),
@@ -2614,39 +2646,30 @@ JavaVMExt::JavaVMExt(Runtime* runtime, bool check_jni, bool verbose_jni)
 }
 
 JavaVMExt::~JavaVMExt() {
+  delete pins_lock;
   delete globals_lock;
   delete weak_globals_lock;
 }
 
 /*
- * Load native code from the specified absolute pathname.  Per the spec,
- * if we've already loaded a library with the specified pathname, we
- * return without doing anything.
- *
- * TODO? for better results we should absolutify the pathname.  For fully
- * correct results we should stat to get the inode and compare that.  The
- * existing implementation is fine so long as everybody is using
- * System.loadLibrary.
- *
- * The library will be associated with the specified class loader.  The JNI
- * spec says we can't load the same library into more than one class loader.
- *
- * Returns "true" on success. On failure, sets *detail to a
- * human-readable description of the error or NULL if no detail is
- * available; ownership of the string is transferred to the caller.
  */
-bool JavaVMExt::LoadNativeLibrary(const std::string& path, ClassLoader* class_loader, char** detail) {
-  *detail = NULL;
+bool JavaVMExt::LoadNativeLibrary(const std::string& path, ClassLoader* class_loader, std::string& detail) {
+  detail.clear();
 
   // See if we've already loaded this library.  If we have, and the class loader
   // matches, return successfully without doing anything.
+  // TODO: for better results we should canonicalize the pathname (or even compare
+  // inodes). This implementation is fine if everybody is using System.loadLibrary.
   SharedLibrary* library = libraries[path];
   if (library != NULL) {
     if (library->GetClassLoader() != class_loader) {
-      LOG(WARNING) << "Shared library \"" << path << "\" already opened by "
-                   << "ClassLoader " << library->GetClassLoader() << "; "
-                   << "can't open in " << class_loader;
-      *detail = strdup("already opened by different ClassLoader");
+      // The library will be associated with class_loader. The JNI
+      // spec says we can't load the same library into more than one
+      // class loader.
+      StringAppendF(&detail, "Shared library \"%s\" already opened by "
+          "ClassLoader %p; can't open in ClassLoader %p",
+          path.c_str(), library->GetClassLoader(), class_loader);
+      LOG(WARNING) << detail;
       return false;
     }
     if (verbose_jni) {
@@ -2654,7 +2677,8 @@ bool JavaVMExt::LoadNativeLibrary(const std::string& path, ClassLoader* class_lo
                 << "ClassLoader " << class_loader << "]";
     }
     if (!library->CheckOnLoadResult(this)) {
-      *detail = strdup("JNI_OnLoad failed before");
+      StringAppendF(&detail, "JNI_OnLoad failed on a previous attempt "
+          "to load \"%s\"", path.c_str());
       return false;
     }
     return true;
@@ -2698,7 +2722,7 @@ bool JavaVMExt::LoadNativeLibrary(const std::string& path, ClassLoader* class_lo
   }
 
   if (handle == NULL) {
-    *detail = strdup(dlerror());
+    detail = dlerror();
     return false;
   }
 
diff --git a/src/jni_internal.h b/src/jni_internal.h
index 6d1ff962da..2b3529cef2 100644
--- a/src/jni_internal.h
+++ b/src/jni_internal.h
@@ -24,24 +24,13 @@ struct JavaVMExt : public JavaVM {
   JavaVMExt(Runtime* runtime, bool check_jni, bool verbose_jni);
   ~JavaVMExt();
 
-  /*
-   * Load native code from the specified absolute pathname.  Per the spec,
-   * if we've already loaded a library with the specified pathname, we
-   * return without doing anything.
+  /**
+   * Loads the given shared library. 'path' is an absolute pathname.
    *
-   * TODO: for better results we should canonicalize the pathname.  For fully
-   * correct results we should stat to get the inode and compare that.  The
-   * existing implementation is fine so long as everybody is using
-   * System.loadLibrary.
-   *
-   * The library will be associated with the specified class loader.  The JNI
-   * spec says we can't load the same library into more than one class loader.
-   *
-   * Returns true on success. On failure, returns false and sets *detail to a
-   * human-readable description of the error or NULL if no detail is
-   * available; ownership of the string is transferred to the caller.
+   * Returns 'true' on success. On failure, sets 'detail' to a
+   * human-readable description of the error.
    */
-  bool LoadNativeLibrary(const std::string& path, ClassLoader* class_loader, char** detail);
+  bool LoadNativeLibrary(const std::string& path, ClassLoader* class_loader, std::string& detail);
 
   Runtime* runtime;
 
@@ -49,6 +38,7 @@ struct JavaVMExt : public JavaVM {
   bool verbose_jni;
 
   // Used to hold references to pinned primitive arrays.
+  Mutex* pins_lock;
   ReferenceTable pin_table;
 
   // JNI global references.
@@ -63,9 +53,10 @@ struct JavaVMExt : public JavaVM {
 };
 
 struct JNIEnvExt : public JNIEnv {
-  JNIEnvExt(Thread* self, bool check_jni);
+  JNIEnvExt(Thread* self, JavaVMExt* vm);
 
   Thread* self;
+  JavaVMExt* vm;
 
   bool check_jni;
 
diff --git a/src/jni_internal_test.cc b/src/jni_internal_test.cc
index 0964b6223b..ddc0408e0a 100644
--- a/src/jni_internal_test.cc
+++ b/src/jni_internal_test.cc
@@ -279,7 +279,7 @@ TEST_F(JniInternalTest, RegisterNatives) {
   env_->UnregisterNatives(jlobject);
 }
 
-#define EXPECT_PRIMITIVE_ARRAY(new_fn, get_region_fn, set_region_fn, scalar_type, expected_class_descriptor) \
+#define EXPECT_PRIMITIVE_ARRAY(new_fn, get_region_fn, set_region_fn, get_elements_fn, release_elements_fn, scalar_type, expected_class_descriptor) \
   jsize size = 4; \
   /* Allocate an array and check it has the right type and length. */ \
   scalar_type ## Array a = env_->new_fn(size); \
@@ -317,31 +317,40 @@ TEST_F(JniInternalTest, RegisterNatives) {
   EXPECT_TRUE(memcmp(src_buf, dst_buf, sizeof(src_buf)) == 0) << "fixed copy not equal"; \
   /* Copy back the whole array. */ \
   env_->get_region_fn(a, 0, size, dst_buf); \
-  EXPECT_TRUE(memcmp(src_buf, dst_buf, sizeof(src_buf)) == 0) << "full copy not equal"
+  EXPECT_TRUE(memcmp(src_buf, dst_buf, sizeof(src_buf)) == 0) << "full copy not equal"; \
+  /* GetPrimitiveArrayCritical */ \
+  void* v = env_->GetPrimitiveArrayCritical(a, NULL); \
+  EXPECT_TRUE(memcmp(src_buf, v, sizeof(src_buf)) == 0) << "GetPrimitiveArrayCritical not equal"; \
+  env_->ReleasePrimitiveArrayCritical(a, v, 0); \
+  /* GetXArrayElements */ \
+  scalar_type* xs = env_->get_elements_fn(a, NULL); \
+  EXPECT_TRUE(memcmp(src_buf, xs, sizeof(src_buf)) == 0) << # get_elements_fn " not equal"; \
+  env_->release_elements_fn(a, xs, 0); \
+  EXPECT_EQ(reinterpret_cast<uintptr_t>(v), reinterpret_cast<uintptr_t>(xs))
 
 TEST_F(JniInternalTest, BooleanArrays) {
-  EXPECT_PRIMITIVE_ARRAY(NewBooleanArray, GetBooleanArrayRegion, SetBooleanArrayRegion, jboolean, "[Z");
+  EXPECT_PRIMITIVE_ARRAY(NewBooleanArray, GetBooleanArrayRegion, SetBooleanArrayRegion, GetBooleanArrayElements, ReleaseBooleanArrayElements, jboolean, "[Z");
 }
 TEST_F(JniInternalTest, ByteArrays) {
-  EXPECT_PRIMITIVE_ARRAY(NewByteArray, GetByteArrayRegion, SetByteArrayRegion, jbyte, "[B");
+  EXPECT_PRIMITIVE_ARRAY(NewByteArray, GetByteArrayRegion, SetByteArrayRegion, GetByteArrayElements, ReleaseByteArrayElements, jbyte, "[B");
 }
 TEST_F(JniInternalTest, CharArrays) {
-  EXPECT_PRIMITIVE_ARRAY(NewCharArray, GetCharArrayRegion, SetCharArrayRegion, jchar, "[C");
+  EXPECT_PRIMITIVE_ARRAY(NewCharArray, GetCharArrayRegion, SetCharArrayRegion, GetCharArrayElements, ReleaseCharArrayElements, jchar, "[C");
 }
 TEST_F(JniInternalTest, DoubleArrays) {
-  EXPECT_PRIMITIVE_ARRAY(NewDoubleArray, GetDoubleArrayRegion, SetDoubleArrayRegion, jdouble, "[D");
+  EXPECT_PRIMITIVE_ARRAY(NewDoubleArray, GetDoubleArrayRegion, SetDoubleArrayRegion, GetDoubleArrayElements, ReleaseDoubleArrayElements, jdouble, "[D");
 }
 TEST_F(JniInternalTest, FloatArrays) {
-  EXPECT_PRIMITIVE_ARRAY(NewFloatArray, GetFloatArrayRegion, SetFloatArrayRegion, jfloat, "[F");
+  EXPECT_PRIMITIVE_ARRAY(NewFloatArray, GetFloatArrayRegion, SetFloatArrayRegion, GetFloatArrayElements, ReleaseFloatArrayElements, jfloat, "[F");
 }
 TEST_F(JniInternalTest, IntArrays) {
-  EXPECT_PRIMITIVE_ARRAY(NewIntArray, GetIntArrayRegion, SetIntArrayRegion, jint, "[I");
+  EXPECT_PRIMITIVE_ARRAY(NewIntArray, GetIntArrayRegion, SetIntArrayRegion, GetIntArrayElements, ReleaseIntArrayElements, jint, "[I");
 }
 TEST_F(JniInternalTest, LongArrays) {
-  EXPECT_PRIMITIVE_ARRAY(NewLongArray, GetLongArrayRegion, SetLongArrayRegion, jlong, "[J");
+  EXPECT_PRIMITIVE_ARRAY(NewLongArray, GetLongArrayRegion, SetLongArrayRegion, GetLongArrayElements, ReleaseLongArrayElements, jlong, "[J");
 }
 TEST_F(JniInternalTest, ShortArrays) {
-  EXPECT_PRIMITIVE_ARRAY(NewShortArray, GetShortArrayRegion, SetShortArrayRegion, jshort, "[S");
+  EXPECT_PRIMITIVE_ARRAY(NewShortArray, GetShortArrayRegion, SetShortArrayRegion, GetShortArrayElements, ReleaseShortArrayElements, jshort, "[S");
 }
 
 TEST_F(JniInternalTest, NewObjectArray) {
@@ -365,6 +374,15 @@ TEST_F(JniInternalTest, NewObjectArray) {
   EXPECT_TRUE(a != NULL);
   EXPECT_TRUE(env_->IsInstanceOf(a, array_class));
   EXPECT_EQ(1, env_->GetArrayLength(a));
+  EXPECT_TRUE(env_->IsSameObject(env_->GetObjectArrayElement(a, 0), NULL));
+
+  jstring s = env_->NewStringUTF("poop");
+  a = env_->NewObjectArray(2, element_class, s);
+  EXPECT_TRUE(a != NULL);
+  EXPECT_TRUE(env_->IsInstanceOf(a, array_class));
+  EXPECT_EQ(2, env_->GetArrayLength(a));
+  EXPECT_TRUE(env_->IsSameObject(env_->GetObjectArrayElement(a, 0), s));
+  EXPECT_TRUE(env_->IsSameObject(env_->GetObjectArrayElement(a, 1), s));
 }
 
 TEST_F(JniInternalTest, GetArrayLength) {
@@ -495,6 +513,71 @@ TEST_F(JniInternalTest, GetStringRegion_GetStringUTFRegion) {
   EXPECT_EQ('x', bytes[3]);
 }
 
+TEST_F(JniInternalTest, GetStringUTFChars_ReleaseStringUTFChars) {
+  EXPECT_TRUE(env_->GetStringUTFChars(NULL, NULL) == NULL);
+
+  jstring s = env_->NewStringUTF("hello");
+  ASSERT_TRUE(s != NULL);
+
+  const char* utf = env_->GetStringUTFChars(s, NULL);
+  EXPECT_STREQ("hello", utf);
+  env_->ReleaseStringUTFChars(s, utf);
+
+  jboolean is_copy = JNI_FALSE;
+  utf = env_->GetStringUTFChars(s, &is_copy);
+  EXPECT_EQ(JNI_TRUE, is_copy);
+  EXPECT_STREQ("hello", utf);
+  env_->ReleaseStringUTFChars(s, utf);
+}
+
+TEST_F(JniInternalTest, GetStringChars_ReleaseStringChars) {
+  jstring s = env_->NewStringUTF("hello");
+  ASSERT_TRUE(s != NULL);
+
+  jchar expected[] = { 'h', 'e', 'l', 'l', 'o' };
+  const jchar* chars = env_->GetStringChars(s, NULL);
+  EXPECT_EQ(expected[0], chars[0]);
+  EXPECT_EQ(expected[1], chars[1]);
+  EXPECT_EQ(expected[2], chars[2]);
+  EXPECT_EQ(expected[3], chars[3]);
+  EXPECT_EQ(expected[4], chars[4]);
+  env_->ReleaseStringChars(s, chars);
+
+  jboolean is_copy = JNI_FALSE;
+  chars = env_->GetStringChars(s, &is_copy);
+  EXPECT_EQ(JNI_FALSE, is_copy);
+  EXPECT_EQ(expected[0], chars[0]);
+  EXPECT_EQ(expected[1], chars[1]);
+  EXPECT_EQ(expected[2], chars[2]);
+  EXPECT_EQ(expected[3], chars[3]);
+  EXPECT_EQ(expected[4], chars[4]);
+  env_->ReleaseStringChars(s, chars);
+}
+
+TEST_F(JniInternalTest, GetStringCritical_ReleaseStringCritical) {
+  jstring s = env_->NewStringUTF("hello");
+  ASSERT_TRUE(s != NULL);
+
+  jchar expected[] = { 'h', 'e', 'l', 'l', 'o' };
+  const jchar* chars = env_->GetStringCritical(s, NULL);
+  EXPECT_EQ(expected[0], chars[0]);
+  EXPECT_EQ(expected[1], chars[1]);
+  EXPECT_EQ(expected[2], chars[2]);
+  EXPECT_EQ(expected[3], chars[3]);
+  EXPECT_EQ(expected[4], chars[4]);
+  env_->ReleaseStringCritical(s, chars);
+
+  jboolean is_copy = JNI_FALSE;
+  chars = env_->GetStringCritical(s, &is_copy);
+  EXPECT_EQ(JNI_FALSE, is_copy);
+  EXPECT_EQ(expected[0], chars[0]);
+  EXPECT_EQ(expected[1], chars[1]);
+  EXPECT_EQ(expected[2], chars[2]);
+  EXPECT_EQ(expected[3], chars[3]);
+  EXPECT_EQ(expected[4], chars[4]);
+  env_->ReleaseStringCritical(s, chars);
+}
+
 TEST_F(JniInternalTest, GetObjectArrayElement_SetObjectArrayElement) {
   jclass c = env_->FindClass("[Ljava/lang/Object;");
   ASSERT_TRUE(c != NULL);
diff --git a/src/reference_table.cc b/src/reference_table.cc
index 0fc038a58f..b5c988aaa5 100644
--- a/src/reference_table.cc
+++ b/src/reference_table.cc
@@ -29,7 +29,7 @@ ReferenceTable::ReferenceTable(const char* name,
   entries_.reserve(initial_size);
 }
 
-void ReferenceTable::Add(Object* obj) {
+void ReferenceTable::Add(const Object* obj) {
   DCHECK(obj != NULL);
   if (entries_.size() == max_size_) {
     LOG(FATAL) << "ReferenceTable '" << name_ << "' "
@@ -38,7 +38,7 @@ void ReferenceTable::Add(Object* obj) {
   entries_.push_back(obj);
 }
 
-void ReferenceTable::Remove(Object* obj) {
+void ReferenceTable::Remove(const Object* obj) {
   // We iterate backwards on the assumption that references are LIFO.
   for (int i = entries_.size() - 1; i >= 0; --i) {
     if (entries_[i] == obj) {
@@ -58,7 +58,7 @@ size_t GetElementCount(const Object* obj) {
 }
 
 struct ObjectComparator {
-  bool operator()(Object* obj1, Object* obj2){
+  bool operator()(const Object* obj1, const Object* obj2){
     // Ensure null references and cleared jweaks appear at the end.
     if (obj1 == NULL) {
       return true;
@@ -132,7 +132,7 @@ void ReferenceTable::Dump() const {
   Dump(entries_);
 }
 
-void ReferenceTable::Dump(const std::vector<Object*>& entries) {
+void ReferenceTable::Dump(const std::vector<const Object*>& entries) {
   if (entries.empty()) {
     LOG(WARNING) << "  (empty)";
     return;
@@ -192,7 +192,7 @@ void ReferenceTable::Dump(const std::vector<Object*>& entries) {
   }
 
   // Make a copy of the table and sort it.
-  std::vector<Object*> sorted_entries(entries.begin(), entries.end());
+  std::vector<const Object*> sorted_entries(entries.begin(), entries.end());
   std::sort(sorted_entries.begin(), sorted_entries.end(), ObjectComparator());
 
   // Remove any uninteresting stuff from the list. The sort moved them all to the end.
@@ -211,8 +211,8 @@ void ReferenceTable::Dump(const std::vector<Object*>& entries) {
   size_t equiv = 0;
   size_t identical = 0;
   for (size_t idx = 1; idx < count; idx++) {
-    Object* prev = sorted_entries[idx-1];
-    Object* current = sorted_entries[idx];
+    const Object* prev = sorted_entries[idx-1];
+    const Object* current = sorted_entries[idx];
     size_t elems = GetElementCount(prev);
     if (current == prev) {
       // Same reference, added more than once.
diff --git a/src/reference_table.h b/src/reference_table.h
index 2e4d95401b..15a2e11e66 100644
--- a/src/reference_table.h
+++ b/src/reference_table.h
@@ -34,20 +34,20 @@ class ReferenceTable {
  public:
   ReferenceTable(const char* name, size_t initial_size, size_t max_size);
 
-  void Add(Object* obj);
+  void Add(const Object* obj);
 
-  void Remove(Object* obj);
+  void Remove(const Object* obj);
 
   size_t Size() const;
 
   void Dump() const;
 
  private:
-  static void Dump(const std::vector<Object*>& entries);
+  static void Dump(const std::vector<const Object*>& entries);
   friend class IndirectReferenceTable; // For Dump.
 
   std::string name_;
-  std::vector<Object*> entries_;
+  std::vector<const Object*> entries_;
   size_t max_size_;
 };
 
diff --git a/src/runtime.cc b/src/runtime.cc
index b009e221d7..ac0b90b4b9 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -155,8 +155,8 @@ size_t ParseMemoryOption(const char *s, size_t div) {
 void LoadJniLibrary(JavaVMExt* vm, const char* name) {
   // TODO: OS_SHARED_LIB_FORMAT_STR
   std::string mapped_name(StringPrintf("lib%s.so", name));
-  char* reason = NULL;
-  if (!vm->LoadNativeLibrary(mapped_name, NULL, &reason)) {
+  std::string reason;
+  if (!vm->LoadNativeLibrary(mapped_name, NULL, reason)) {
     LOG(FATAL) << "LoadNativeLibrary failed for \"" << mapped_name << "\": "
                << reason;
   }
@@ -356,12 +356,10 @@ bool Runtime::Init(const Options& raw_options, bool ignore_unrecognized) {
   return true;
 }
 
-bool Runtime::AttachCurrentThread(const char* name, JNIEnv** penv) {
-  return Thread::Attach(instance_) != NULL;
-}
-
-bool Runtime::AttachCurrentThreadAsDaemon(const char* name, JNIEnv** penv) {
-  // TODO: do something different for daemon threads.
+bool Runtime::AttachCurrentThread(const char* name, JNIEnv** penv, bool as_daemon) {
+  if (as_daemon) {
+    UNIMPLEMENTED(WARNING) << "TODO: do something different for daemon threads";
+  }
   return Thread::Attach(instance_) != NULL;
 }
 
diff --git a/src/runtime.h b/src/runtime.h
index 9879023512..49657c238a 100644
--- a/src/runtime.h
+++ b/src/runtime.h
@@ -71,8 +71,7 @@ class Runtime {
   static void Abort(const char* file, int line);
 
   // Attaches the current native thread to the runtime.
-  bool AttachCurrentThread(const char* name, JNIEnv** jni_env);
-  bool AttachCurrentThreadAsDaemon(const char* name, JNIEnv** jni_env);
+  bool AttachCurrentThread(const char* name, JNIEnv** jni_env, bool as_daemon);
 
   // Detaches the current native thread from the runtime.
   bool DetachCurrentThread();
diff --git a/src/thread.cc b/src/thread.cc
index 477b14974c..0878b50173 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -175,10 +175,7 @@ Thread* Thread::Attach(const Runtime* runtime) {
       PLOG(FATAL) << "pthread_setspecific failed";
   }
 
-  JavaVMExt* vm = runtime->GetJavaVM();
-  CHECK(vm != NULL);
-  bool check_jni = vm->check_jni;
-  thread->jni_env_ = new JNIEnvExt(thread, check_jni);
+  thread->jni_env_ = new JNIEnvExt(thread, runtime->GetJavaVM());
 
   return thread;
 }