Commit c9cebff9 authored by Jacob Potter's avatar Jacob Potter

Use Java WeakRef insteaed of JNI WeakGlobalRef

This prevents a race condition against the garbage collector.
parent d1153188
...@@ -75,14 +75,6 @@ static JNIEnv * getOptThreadEnv() { ...@@ -75,14 +75,6 @@ static JNIEnv * getOptThreadEnv() {
return env; return env;
} }
void WeakGlobalRefDeleter::operator() (jobject wgr) noexcept {
if (wgr) {
if (JNIEnv * env = getOptThreadEnv()) {
env->DeleteWeakGlobalRef(wgr);
}
}
}
void GlobalRefDeleter::operator() (jobject globalRef) noexcept { void GlobalRefDeleter::operator() (jobject globalRef) noexcept {
if (globalRef) { if (globalRef) {
if (JNIEnv * env = getOptThreadEnv()) { if (JNIEnv * env = getOptThreadEnv()) {
...@@ -472,9 +464,49 @@ CppProxyClassInfo::CppProxyClassInfo() : constructor{}, idField{} { ...@@ -472,9 +464,49 @@ CppProxyClassInfo::CppProxyClassInfo() : constructor{}, idField{} {
CppProxyClassInfo::~CppProxyClassInfo() { CppProxyClassInfo::~CppProxyClassInfo() {
} }
/*
* Wrapper around Java WeakReference objects. (We can't use JNI NewWeakGlobalRef() because
* it doesn't have the right semantics - see comment in djinni_support.hpp.)
*/
class JavaWeakRef {
private:
struct JniInfo {
public:
const GlobalRef<jclass> clazz { jniFindClass("java/lang/ref/WeakReference") };
const jmethodID constructor { jniGetMethodID(clazz.get(), "<init>", "(Ljava/lang/Object;)V") };
const jmethodID method_get { jniGetMethodID(clazz.get(), "get", "()Ljava/lang/Object;") };
};
// Helper used by constructor
static jobject create(JNIEnv * jniEnv, jobject obj) {
const JniInfo & weakRefClass = JniClass<JniInfo>::get();
jobject weakRef = jniEnv->NewObject(weakRefClass.clazz.get(), weakRefClass.constructor, obj);
// DJINNI_ASSERT performs an exception check before anything else, so we don't need
// a separate jniExceptionCheck call.
DJINNI_ASSERT(weakRef, jniEnv);
return weakRef;
}
public:
// Constructor
JavaWeakRef(JNIEnv * jniEnv, jobject obj) : m_weakRef(jniEnv, create(jniEnv, obj)) {}
// Get the object pointed to if it's still strongly reachable or, return null if not.
// (Analogous to weak_ptr::lock.) Returns a local reference.
jobject get(JNIEnv * jniEnv) {
const JniInfo & weakRefClass = JniClass<JniInfo>::get();
jobject javaObj = jniEnv->CallObjectMethod(m_weakRef.get(), weakRefClass.method_get);
jniExceptionCheck(jniEnv);
return javaObj;
}
private:
GlobalRef<jobject> m_weakRef;
};
struct CppProxyCacheState { struct CppProxyCacheState {
std::mutex mtx; std::mutex mtx;
std::unordered_map<void *, WeakGlobalRef<jobject>> m; std::unordered_map<void *, JavaWeakRef> m;
static CppProxyCacheState & get() { static CppProxyCacheState & get() {
static CppProxyCacheState st; static CppProxyCacheState st;
...@@ -499,14 +531,16 @@ struct CppProxyCacheState { ...@@ -499,14 +531,16 @@ struct CppProxyCacheState {
auto it = st.m.find(cppObj.get()); auto it = st.m.find(cppObj.get());
if (it != st.m.end()) { if (it != st.m.end()) {
jobject localRef = jniEnv->NewLocalRef(it->second.get()); // It's in the map. See if the WeakReference still points to an object.
if (localRef) { if (jobject javaObj = it->second.get(jniEnv)) {
return localRef; return javaObj;
} }
} }
jobject wrapper = factory(cppObj, jniEnv, proxyClass); jobject wrapper = factory(cppObj, jniEnv, proxyClass);
st.m.emplace(cppObj.get(), WeakGlobalRef<jobject>(jniEnv, wrapper));
/* Make a Java WeakRef object */
st.m.emplace(cppObj.get(), JavaWeakRef(jniEnv, wrapper));
return wrapper; return wrapper;
} }
......
...@@ -107,20 +107,6 @@ public: ...@@ -107,20 +107,6 @@ public:
localRef) {} localRef) {}
}; };
struct WeakGlobalRefDeleter { void operator() (jobject wgr) noexcept; };
template <typename PointerType>
class WeakGlobalRef : public std::unique_ptr<typename std::remove_pointer<PointerType>::type,
WeakGlobalRefDeleter> {
public:
WeakGlobalRef() {}
WeakGlobalRef(JNIEnv * env, PointerType localRef)
: std::unique_ptr<typename std::remove_pointer<PointerType>::type, WeakGlobalRefDeleter>(
static_cast<PointerType>(env->NewWeakGlobalRef(localRef)),
WeakGlobalRefDeleter{}
) {}
};
/* /*
* Helper for JniClassInitializer. Copied from Oxygen. * Helper for JniClassInitializer. Copied from Oxygen.
*/ */
...@@ -307,12 +293,17 @@ public: ...@@ -307,12 +293,17 @@ public:
* | | | | | | | * | | | | | | |
* | Foo.CppProxy | ------------> | CppProxyHandle | =============> | Foo | * | Foo.CppProxy | ------------> | CppProxyHandle | =============> | Foo |
* |______________| (jlong) | <Foo> | (shared_ptr) |___________| * |______________| (jlong) | <Foo> | (shared_ptr) |___________|
* ^ | |________________| * ^ | |________________|
* \ | * \ |
* \ | __________________ * _________ | __________________
* -----------------------| | * | | | | |
* (WeakGlobalRef) | jniCppProxyCache | * | WeakRef | <------------------------- | jniCppProxyCache |
* | |__________________| * |_________| (GlobalRef) |__________________|
* |
*
* We don't use JNI WeakGlobalRef objects, because they last longer than is safe - a
* WeakGlobalRef can still be upgraded to a strong reference even during finalization, which
* leads to use-after-free. Java WeakRefs provide the right lifetime guarantee.
*/ */
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment