From a16c1056275e63c168fa7b74d7db6cf679fadc6e Mon Sep 17 00:00:00 2001 From: yunhanw Date: Thu, 12 Oct 2023 19:10:30 -0700 Subject: [PATCH] add JniGlobalReference class with move constructor --- src/controller/java/AndroidCallbacks.cpp | 47 +++++++----------------- src/controller/java/AndroidCallbacks.h | 9 +++-- src/lib/support/JniTypeWrappers.h | 34 +++++++++++++++-- 3 files changed, 50 insertions(+), 40 deletions(-) diff --git a/src/controller/java/AndroidCallbacks.cpp b/src/controller/java/AndroidCallbacks.cpp index 93df488d2d593f..53d3b2d760eb08 100644 --- a/src/controller/java/AndroidCallbacks.cpp +++ b/src/controller/java/AndroidCallbacks.cpp @@ -26,7 +26,6 @@ #include #include #include -#include #include #include #include @@ -76,11 +75,7 @@ GetConnectedDeviceCallback::GetConnectedDeviceCallback(jobject wrapperCallback, { JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread(); VerifyOrReturn(env != nullptr, ChipLogError(Controller, "Could not get JNIEnv for current thread")); - mWrapperCallbackRef = env->NewGlobalRef(wrapperCallback); - if (mWrapperCallbackRef == nullptr) - { - ChipLogError(Controller, "Could not create global reference for Java callback"); - } + VerifyOrReturn(mWrapperCallbackRef.Init(env, wrapperCallback) != CHIP_NO_ERROR, ChipLogError(Controller, "Could not init mWrapperCallbackRef for GetConnectedDeviceCallback")); mJavaCallbackRef = env->NewGlobalRef(javaCallback); if (mJavaCallbackRef == nullptr) { @@ -107,8 +102,7 @@ void GetConnectedDeviceCallback::OnDeviceConnectedFn(void * context, Messaging:: jobject javaCallback = self->mJavaCallbackRef; JniLocalReferenceManager manager(env); // Release wrapper's global ref so application can clean up the actual callback underneath. - VerifyOrReturn(self->mWrapperCallbackRef != nullptr, ChipLogError(Controller, "mWrapperCallbackRef is null")); - JniObject wrapper(env, self->mWrapperCallbackRef); + JniGlobalReference globalRef(std::move(self->mWrapperCallbackRef)); jclass getConnectedDeviceCallbackCls = nullptr; JniReferences::GetInstance().GetLocalClassRef( @@ -178,11 +172,9 @@ ReportCallback::ReportCallback(jobject wrapperCallback, jobject subscriptionEsta { ChipLogError(Controller, "Could not create global reference for Java callback"); } - mWrapperCallbackRef = env->NewGlobalRef(wrapperCallback); - if (mWrapperCallbackRef == nullptr) - { - ChipLogError(Controller, "Could not create global reference for Java callback"); - } + + VerifyOrReturn(mWrapperCallbackRef.Init(env, wrapperCallback) != CHIP_NO_ERROR, ChipLogError(Controller, "Could not init mWrapperCallbackRef for ReportCallback")); + if (resubscriptionAttemptCallback != nullptr) { mResubscriptionAttemptCallbackRef = env->NewGlobalRef(resubscriptionAttemptCallback); @@ -591,8 +583,8 @@ void ReportCallback::OnDone(app::ReadClient *) JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread(); VerifyOrReturn(env != nullptr, ChipLogError(Controller, "Could not get JNIEnv for current thread")); JniLocalReferenceManager manager(env); - VerifyOrReturn(mWrapperCallbackRef != nullptr, ChipLogError(Controller, "mWrapperCallbackRef is null")); - JniObject wrapper(env, mWrapperCallbackRef); + VerifyOrReturn(mWrapperCallbackRef.HasValidObjectRef(), ChipLogError(Controller, "mWrapperCallbackRef is nullptr in ReportCallback::OnDone")); + JniGlobalReference globalRef(std::move(mWrapperCallbackRef)); jmethodID onDoneMethod; err = JniReferences::GetInstance().FindMethod(env, mReportCallbackRef, "onDone", "()V", &onDoneMethod); VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Controller, "Could not find onDone method")); @@ -677,13 +669,7 @@ WriteAttributesCallback::WriteAttributesCallback(jobject wrapperCallback, jobjec JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread(); VerifyOrReturn(env != nullptr, ChipLogError(Controller, "Could not get JNIEnv for current thread")); - mWrapperCallbackRef = env->NewGlobalRef(wrapperCallback); - VerifyOrReturn(!env->ExceptionCheck(), env->ExceptionDescribe()); - if (mWrapperCallbackRef == nullptr) - { - ChipLogError(Controller, "Could not create global reference for Wrapper WriteAttributesCallback"); - return; - } + VerifyOrReturn(mWrapperCallbackRef.Init(env, wrapperCallback) != CHIP_NO_ERROR, ChipLogError(Controller, "Could not init mWrapperCallbackRef for WriteAttributesCallback")); mJavaCallbackRef = env->NewGlobalRef(javaCallback); VerifyOrReturn(!env->ExceptionCheck(), env->ExceptionDescribe()); if (mJavaCallbackRef == nullptr) @@ -747,8 +733,8 @@ void WriteAttributesCallback::OnDone(app::WriteClient *) JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread(); VerifyOrReturn(env != nullptr, ChipLogError(Controller, "Could not get JNIEnv for current thread")); JniLocalReferenceManager manager(env); - VerifyOrReturn(mWrapperCallbackRef != nullptr, ChipLogError(Controller, "mWrapperCallbackRef is null")); - JniObject wrapper(env, mWrapperCallbackRef); + VerifyOrReturn(mWrapperCallbackRef.HasValidObjectRef(), ChipLogError(Controller, "mWrapperCallbackRef is nullptr in WriteAttributesCallback::OnDone")); + JniGlobalReference globalRef(std::move(mWrapperCallbackRef)); jmethodID onDoneMethod; err = JniReferences::GetInstance().FindMethod(env, mJavaCallbackRef, "onDone", "()V", &onDoneMethod); VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Controller, "Could not find onDone method")); @@ -796,13 +782,8 @@ InvokeCallback::InvokeCallback(jobject wrapperCallback, jobject javaCallback) { JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread(); VerifyOrReturn(env != nullptr, ChipLogError(Controller, "Could not get JNIEnv for current thread")); - mWrapperCallbackRef = env->NewGlobalRef(wrapperCallback); - VerifyOrReturn(!env->ExceptionCheck(), env->ExceptionDescribe()); - if (mWrapperCallbackRef == nullptr) - { - ChipLogError(Controller, "Could not create global reference for Wrapper InvokeCallback"); - return; - } + VerifyOrReturn(mWrapperCallbackRef.Init(env, wrapperCallback) != CHIP_NO_ERROR, ChipLogError(Controller, "Could not init mWrapperCallbackRef for InvokeCallback")); + mJavaCallbackRef = env->NewGlobalRef(javaCallback); VerifyOrReturn(!env->ExceptionCheck(), env->ExceptionDescribe()); if (mJavaCallbackRef == nullptr) @@ -867,8 +848,8 @@ void InvokeCallback::OnDone(app::CommandSender * apCommandSender) JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread(); VerifyOrReturn(env != nullptr, ChipLogError(Controller, "Could not get JNIEnv for current thread")); JniLocalReferenceManager manager(env); - VerifyOrReturn(mWrapperCallbackRef != nullptr, ChipLogError(Controller, "mWrapperCallbackRef is null")); - JniObject wrapper(env, mWrapperCallbackRef); + VerifyOrReturn(mWrapperCallbackRef.HasValidObjectRef(), ChipLogError(Controller, "mWrapperCallbackRef is nullptr in InvokeCallback::OnDone")); + JniGlobalReference globalRef(std::move(mWrapperCallbackRef)); jmethodID onDoneMethod; err = JniReferences::GetInstance().FindMethod(env, mJavaCallbackRef, "onDone", "()V", &onDoneMethod); VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Controller, "Could not find onDone method")); diff --git a/src/controller/java/AndroidCallbacks.h b/src/controller/java/AndroidCallbacks.h index 69e414c124bb4e..39551d8a224cca 100644 --- a/src/controller/java/AndroidCallbacks.h +++ b/src/controller/java/AndroidCallbacks.h @@ -25,6 +25,7 @@ #include #include #include +#include namespace chip { namespace Controller { @@ -42,7 +43,7 @@ struct GetConnectedDeviceCallback Callback::Callback mOnSuccess; Callback::Callback mOnFailure; - jobject mWrapperCallbackRef = nullptr; + JniGlobalReference mWrapperCallbackRef; jobject mJavaCallbackRef = nullptr; }; @@ -83,7 +84,7 @@ struct ReportCallback : public app::ClusterStateCache::Callback app::ReadClient * mReadClient = nullptr; app::ClusterStateCache mClusterCacheAdapter; - jobject mWrapperCallbackRef = nullptr; + JniGlobalReference mWrapperCallbackRef; jobject mSubscriptionEstablishedCallbackRef = nullptr; jobject mResubscriptionAttemptCallbackRef = nullptr; jobject mReportCallbackRef = nullptr; @@ -110,7 +111,7 @@ struct WriteAttributesCallback : public app::WriteClient::Callback app::WriteClient * mWriteClient = nullptr; app::ChunkedWriteCallback mChunkedWriteCallback; - jobject mWrapperCallbackRef = nullptr; + JniGlobalReference mWrapperCallbackRef; jobject mJavaCallbackRef = nullptr; }; @@ -132,7 +133,7 @@ struct InvokeCallback : public app::CommandSender::Callback void ReportError(const char * message, ChipError::StorageType errorCode); app::CommandSender * mCommandSender = nullptr; - jobject mWrapperCallbackRef = nullptr; + JniGlobalReference mWrapperCallbackRef; jobject mJavaCallbackRef = nullptr; }; diff --git a/src/lib/support/JniTypeWrappers.h b/src/lib/support/JniTypeWrappers.h index 48d2648db268be..3170eef37e48f7 100644 --- a/src/lib/support/JniTypeWrappers.h +++ b/src/lib/support/JniTypeWrappers.h @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -203,11 +204,33 @@ class JniLocalReferenceManager bool mlocalFramePushed = false; }; -class JniObject +class JniGlobalReference { public: - JniObject(JNIEnv * aEnv, jobject aObjectRef) : mEnv(aEnv), mObjectRef(aObjectRef) {} - ~JniObject() + JniGlobalReference() {} + + CHIP_ERROR Init(JNIEnv * aEnv, jobject aObjectRef) + { + VerifyOrReturnError(mEnv != nullptr && mObjectRef != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(aEnv != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + mEnv = aEnv; + mObjectRef = mEnv->NewGlobalRef(aObjectRef); + VerifyOrReturnError(!mEnv->ExceptionCheck(), CHIP_JNI_ERROR_EXCEPTION_THROWN); + VerifyOrReturnError(mObjectRef != nullptr, CHIP_JNI_ERROR_NULL_OBJECT); + return CHIP_NO_ERROR; + } + + JniGlobalReference(JniGlobalReference && aOther) { + // Assign the other object's JNI environment and object reference to our own. + mEnv = aOther.mEnv; + mObjectRef = aOther.mObjectRef; + + // Set the other object's JNI environment and object reference to nullptr to prevent it from freeing the resources twice. + aOther.mEnv = nullptr; + aOther.mObjectRef = nullptr; + } + + ~JniGlobalReference() { if (mEnv != nullptr && mObjectRef != nullptr) { @@ -217,6 +240,11 @@ class JniObject jobject objectRef() { return mObjectRef; } + bool HasValidObjectRef() + { + return mObjectRef != nullptr; + } + private: JNIEnv * mEnv = nullptr; jobject mObjectRef = nullptr;