Add back the work-arounds for broken apps that assume JNI uses direct references.
Note that we'll need to add support for this to jni_compiler too.
Change-Id: I7e31ab4314ba913cde4629544addd0aad9a89b3b
diff --git a/src/check_jni.cc b/src/check_jni.cc
index 130acef..93d3b31 100644
--- a/src/check_jni.cc
+++ b/src/check_jni.cc
@@ -21,6 +21,7 @@
#include "class_linker.h"
#include "logging.h"
+#include "scoped_jni_thread_state.h"
#include "thread.h"
namespace art {
@@ -52,50 +53,6 @@
* ===========================================================================
*/
-// TODO: remove this ODR violation!
-class ScopedJniThreadState {
- public:
- explicit ScopedJniThreadState(JNIEnv* env)
- : env_(reinterpret_cast<JNIEnvExt*>(env)) {
- self_ = ThreadForEnv(env);
- self_->SetState(Thread::kRunnable);
- }
-
- ~ScopedJniThreadState() {
- self_->SetState(Thread::kNative);
- }
-
- JNIEnvExt* Env() {
- return env_;
- }
-
- Thread* Self() {
- return self_;
- }
-
- JavaVMExt* Vm() {
- return env_->vm;
- }
-
- private:
- static Thread* ThreadForEnv(JNIEnv* env) {
- // TODO: need replacement for gDvmJni.
- bool workAroundAppJniBugs = true;
- Thread* env_self = reinterpret_cast<JNIEnvExt*>(env)->self;
- Thread* self = workAroundAppJniBugs ? Thread::Current() : env_self;
- if (self != env_self) {
- LOG(ERROR) << "JNI ERROR: JNIEnv for " << *env_self
- << " used on " << *self;
- // TODO: dump stack
- }
- return self;
- }
-
- JNIEnvExt* env_;
- Thread* self_;
- DISALLOW_COPY_AND_ASSIGN(ScopedJniThreadState);
-};
-
template<typename T>
T Decode(ScopedJniThreadState& ts, jobject obj) {
return reinterpret_cast<T>(ts.Self()->DecodeJObject(obj));
@@ -802,7 +759,7 @@
Object* o = Decode<Object*>(ts, java_object);
if (o != NULL && !Heap::IsHeapAddress(o)) {
- // TODO: when we remove workAroundAppJniBugs, this should be impossible.
+ // TODO: when we remove work_around_app_jni_bugs, this should be impossible.
LOG(ERROR) << "JNI ERROR: native code passing in reference to invalid " << GetIndirectRefKind(java_object) << ": " << java_object;
JniAbort();
}
@@ -837,7 +794,7 @@
if (mEnv != threadEnv) {
LOG(ERROR) << "JNI ERROR: thread " << *self << " using JNIEnv* from thread " << *mEnv->self;
// If we're keeping broken code limping along, we need to suppress the abort...
- if (true/* TODO: !gDvmJni.workAroundAppJniBugs*/) {
+ if (!mEnv->work_around_app_jni_bugs) {
JniAbort();
return;
}
diff --git a/src/indirect_reference_table.cc b/src/indirect_reference_table.cc
index 6e5446f..083a499 100644
--- a/src/indirect_reference_table.cc
+++ b/src/indirect_reference_table.cc
@@ -228,11 +228,10 @@
DCHECK_GE(segmentState.parts.numHoles, prevState.parts.numHoles);
int idx = ExtractIndex(iref);
- bool workAroundAppJniBugs = false;
- if (GetIndirectRefKind(iref) == kSirtOrInvalid /*&& gDvmJni.workAroundAppJniBugs*/) { // TODO
+ JavaVMExt* vm = Runtime::Current()->GetJavaVM();
+ if (GetIndirectRefKind(iref) == kSirtOrInvalid || vm->work_around_app_jni_bugs) {
idx = LinearScan(iref, bottomIndex, topIndex, table_);
- workAroundAppJniBugs = true;
if (idx == -1) {
LOG(WARNING) << "trying to work around app JNI bugs, but didn't find " << iref << " in table!";
return false;
@@ -253,7 +252,7 @@
if (idx == topIndex-1) {
// Top-most entry. Scan up and consume holes.
- if (workAroundAppJniBugs == false && !CheckEntry("remove", iref, idx)) {
+ if (!vm->work_around_app_jni_bugs && !CheckEntry("remove", iref, idx)) {
return false;
}
@@ -284,7 +283,7 @@
LOG(INFO) << "--- WEIRD: removing null entry " << idx;
return false;
}
- if (workAroundAppJniBugs == false && !CheckEntry("remove", iref, idx)) {
+ if (!vm->work_around_app_jni_bugs && !CheckEntry("remove", iref, idx)) {
return false;
}
diff --git a/src/indirect_reference_table.h b/src/indirect_reference_table.h
index 4a8833c..86c1293 100644
--- a/src/indirect_reference_table.h
+++ b/src/indirect_reference_table.h
@@ -275,7 +275,7 @@
return table_[ExtractIndex(iref)];
}
- // TODO: only used for workAroundAppJniBugs support.
+ // TODO: remove when we remove work_around_app_jni_bugs support.
bool Contains(IndirectRef iref) const;
/*
diff --git a/src/jni_internal.cc b/src/jni_internal.cc
index cc1d8db..9213056 100644
--- a/src/jni_internal.cc
+++ b/src/jni_internal.cc
@@ -18,6 +18,7 @@
#include "logging.h"
#include "object.h"
#include "runtime.h"
+#include "scoped_jni_thread_state.h"
#include "stringpiece.h"
#include "thread.h"
@@ -50,54 +51,6 @@
namespace {
-// Entry/exit processing for all JNI calls.
-//
-// This performs the necessary thread state switching, lets us amortize the
-// cost of working out the current thread, and lets us check (and repair) apps
-// that are using a JNIEnv on the wrong thread.
-class ScopedJniThreadState {
- public:
- explicit ScopedJniThreadState(JNIEnv* env)
- : env_(reinterpret_cast<JNIEnvExt*>(env)) {
- self_ = ThreadForEnv(env);
- self_->SetState(Thread::kRunnable);
- }
-
- ~ScopedJniThreadState() {
- self_->SetState(Thread::kNative);
- }
-
- JNIEnvExt* Env() {
- return env_;
- }
-
- Thread* Self() {
- return self_;
- }
-
- JavaVMExt* Vm() {
- return env_->vm;
- }
-
- private:
- static Thread* ThreadForEnv(JNIEnv* env) {
- // TODO: need replacement for gDvmJni.
- bool workAroundAppJniBugs = true;
- Thread* env_self = reinterpret_cast<JNIEnvExt*>(env)->self;
- Thread* self = workAroundAppJniBugs ? Thread::Current() : env_self;
- if (self != env_self) {
- LOG(ERROR) << "JNI ERROR: JNIEnv for " << *env_self
- << " used on " << *self;
- // TODO: dump stack
- }
- return self;
- }
-
- JNIEnvExt* env_;
- Thread* self_;
- DISALLOW_COPY_AND_ASSIGN(ScopedJniThreadState);
-};
-
/*
* Add a local reference for an object to the current stack frame. When
* the native function returns, the reference will be discarded.
@@ -142,7 +95,7 @@
}
#endif
- if (false /*gDvmJni.workAroundAppJniBugs*/) { // TODO
+ if (ts.Env()->work_around_app_jni_bugs) {
// Hand out direct pointers to support broken old apps.
return reinterpret_cast<T>(obj);
}
@@ -2313,10 +2266,13 @@
return JNILocalRefType;
}
+ if (!ts.Env()->work_around_app_jni_bugs) {
+ return JNIInvalidRefType;
+ }
+
// If we're handing out direct pointers, check whether it's a direct pointer
// to a local reference.
- // TODO: replace 'false' with the replacement for gDvmJni.workAroundAppJniBugs
- if (false && Decode<Object*>(ts, java_object) == reinterpret_cast<Object*>(java_object)) {
+ if (Decode<Object*>(ts, java_object) == reinterpret_cast<Object*>(java_object)) {
if (ts.Env()->locals.Contains(java_object)) {
return JNILocalRefType;
}
@@ -2573,6 +2529,7 @@
: self(self),
vm(vm),
check_jni(vm->check_jni),
+ work_around_app_jni_bugs(vm->work_around_app_jni_bugs),
critical(false),
monitors("monitors", kMonitorsInitial, kMonitorsMax),
locals(kLocalsInitial, kLocalsMax, kLocal) {
@@ -2698,7 +2655,8 @@
check_jni_abort_hook(NULL),
check_jni(check_jni),
verbose_jni(verbose_jni),
- force_copy(false), // TODO
+ force_copy(false), // TODO: add a way to enable this
+ work_around_app_jni_bugs(false), // TODO: add a way to enable this
pins_lock(Mutex::Create("JNI pin table lock")),
pin_table("pin table", kPinTableInitialSize, kPinTableMaxSize),
globals_lock(Mutex::Create("JNI global reference table lock")),
diff --git a/src/jni_internal.h b/src/jni_internal.h
index a4362d3..05df3e4 100644
--- a/src/jni_internal.h
+++ b/src/jni_internal.h
@@ -49,6 +49,9 @@
bool verbose_jni;
bool force_copy;
+ // Used to provide compatibility for apps that assumed direct references.
+ bool work_around_app_jni_bugs;
+
// Used to hold references to pinned primitive arrays.
Mutex* pins_lock;
ReferenceTable pin_table;
@@ -75,7 +78,9 @@
Thread* self;
JavaVMExt* vm;
+ // Frequently-accessed fields cached from JavaVM.
bool check_jni;
+ bool work_around_app_jni_bugs;
// How many nested "critical" JNI calls are we in?
int critical;
diff --git a/src/scoped_jni_thread_state.h b/src/scoped_jni_thread_state.h
new file mode 100644
index 0000000..dd39cc3
--- /dev/null
+++ b/src/scoped_jni_thread_state.h
@@ -0,0 +1,55 @@
+// Copyright 2011 Google Inc. All Rights Reserved.
+// Author: enh@google.com (Elliott Hughes)
+
+#include "jni_internal.h"
+
+namespace art {
+
+// Entry/exit processing for all JNI calls.
+//
+// This performs the necessary thread state switching, lets us amortize the
+// cost of working out the current thread, and lets us check (and repair) apps
+// that are using a JNIEnv on the wrong thread.
+class ScopedJniThreadState {
+ public:
+ explicit ScopedJniThreadState(JNIEnv* env)
+ : env_(reinterpret_cast<JNIEnvExt*>(env)) {
+ self_ = ThreadForEnv(env);
+ self_->SetState(Thread::kRunnable);
+ }
+
+ ~ScopedJniThreadState() {
+ self_->SetState(Thread::kNative);
+ }
+
+ JNIEnvExt* Env() {
+ return env_;
+ }
+
+ Thread* Self() {
+ return self_;
+ }
+
+ JavaVMExt* Vm() {
+ return env_->vm;
+ }
+
+ private:
+ static Thread* ThreadForEnv(JNIEnv* env) {
+ JNIEnvExt* full_env(reinterpret_cast<JNIEnvExt*>(env));
+ Thread* env_self = full_env->self;
+ Thread* self = full_env->work_around_app_jni_bugs ? Thread::Current() : env_self;
+ if (self != env_self) {
+ LOG(ERROR) << "JNI ERROR: JNIEnv for " << *env_self
+ << " used on " << *self;
+ // TODO: dump stack
+ }
+ return self;
+ }
+
+ JNIEnvExt* env_;
+ Thread* self_;
+ DISALLOW_COPY_AND_ASSIGN(ScopedJniThreadState);
+};
+
+} // namespace art
diff --git a/src/thread.cc b/src/thread.cc
index c90c745..9d2862d 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -299,7 +299,7 @@
// Check if this is a local reference in the SIRT
if (SirtContains(obj)) {
result = *reinterpret_cast<Object**>(obj); // Read from SIRT
- } else if (false /*gDvmJni.workAroundAppJniBugs*/) { // TODO
+ } else if (jni_env_->work_around_app_jni_bugs) {
// Assume an invalid local reference is actually a direct pointer.
result = reinterpret_cast<Object*>(obj);
} else {
