From 6d4647d41cfff9cc2300c1bd675304fd95f1137a Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 May 2021 04:35:55 -0400 Subject: [PATCH] Fix android KVS implementation to work across threads (#6372) --- .../java/AndroidKeyValueStoreManagerImpl.cpp | 68 +++++++++++++------ .../java/AndroidKeyValueStoreManagerImpl.h | 6 +- .../java/CHIPDeviceController-JNI.cpp | 2 +- 3 files changed, 52 insertions(+), 24 deletions(-) diff --git a/src/controller/java/AndroidKeyValueStoreManagerImpl.cpp b/src/controller/java/AndroidKeyValueStoreManagerImpl.cpp index d806eaf6b6b00a..b3099bcb817ef5 100644 --- a/src/controller/java/AndroidKeyValueStoreManagerImpl.cpp +++ b/src/controller/java/AndroidKeyValueStoreManagerImpl.cpp @@ -42,18 +42,20 @@ KeyValueStoreManagerImpl KeyValueStoreManagerImpl::sInstance; CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t value_size, size_t * read_bytes_size, size_t offset) { - ReturnErrorCodeIf(mEnv == nullptr, CHIP_ERROR_INCORRECT_STATE); ReturnErrorCodeIf(mKeyValueStoreManagerClass == nullptr, CHIP_ERROR_INCORRECT_STATE); ReturnErrorCodeIf(mGetMethod == nullptr, CHIP_ERROR_INCORRECT_STATE); ReturnErrorCodeIf(offset != 0, CHIP_ERROR_INVALID_ARGUMENT); - UtfString javaKey(mEnv, key); + JNIEnv * env = GetEnvForCurrentThread(); + ReturnErrorCodeIf(env == nullptr, CHIP_ERROR_INTERNAL); - jobject javaValue = mEnv->CallStaticObjectMethod(mKeyValueStoreManagerClass, mGetMethod, javaKey.jniValue()); - if (mEnv->ExceptionCheck()) + UtfString javaKey(env, key); + + jobject javaValue = env->CallStaticObjectMethod(mKeyValueStoreManagerClass, mGetMethod, javaKey.jniValue()); + if (env->ExceptionCheck()) { ChipLogError(DeviceLayer, "Java exception in KVS::Get"); - mEnv->ExceptionDescribe(); + env->ExceptionDescribe(); return CHIP_JNI_ERROR_EXCEPTION_THROWN; } @@ -62,7 +64,7 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t return CHIP_ERROR_KEY_NOT_FOUND; } - JniUtfString utfValue(mEnv, (jstring) javaValue); + JniUtfString utfValue(env, (jstring) javaValue); if (strlen(utfValue.c_str()) > kMaxKvsValueEncodedChars) { ChipLogError(DeviceLayer, "Unexpected large value received from KVS"); @@ -92,19 +94,21 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t CHIP_ERROR KeyValueStoreManagerImpl::_Delete(const char * key) { - ReturnErrorCodeIf(mEnv == nullptr, CHIP_ERROR_INCORRECT_STATE); ReturnErrorCodeIf(mKeyValueStoreManagerClass == nullptr, CHIP_ERROR_INCORRECT_STATE); ReturnErrorCodeIf(mDeleteMethod == nullptr, CHIP_ERROR_INCORRECT_STATE); - UtfString javaKey(mEnv, key); + JNIEnv * env = GetEnvForCurrentThread(); + ReturnErrorCodeIf(env == nullptr, CHIP_ERROR_INTERNAL); + + UtfString javaKey(env, key); - mEnv->CallStaticVoidMethod(mKeyValueStoreManagerClass, mDeleteMethod, javaKey.jniValue()); + env->CallStaticVoidMethod(mKeyValueStoreManagerClass, mDeleteMethod, javaKey.jniValue()); - if (mEnv->ExceptionCheck()) + if (env->ExceptionCheck()) { ChipLogError(DeviceLayer, "Java exception in KVS::Delete"); - mEnv->ExceptionDescribe(); - mEnv->ExceptionClear(); + env->ExceptionDescribe(); + env->ExceptionClear(); return CHIP_JNI_ERROR_EXCEPTION_THROWN; } @@ -113,36 +117,58 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Delete(const char * key) CHIP_ERROR KeyValueStoreManagerImpl::_Put(const char * key, const void * value, size_t value_size) { - ReturnErrorCodeIf(mEnv == nullptr, CHIP_ERROR_INCORRECT_STATE); ReturnErrorCodeIf(mKeyValueStoreManagerClass == nullptr, CHIP_ERROR_INCORRECT_STATE); ReturnErrorCodeIf(mSetMethod == nullptr, CHIP_ERROR_INCORRECT_STATE); ReturnErrorCodeIf(value_size > kMaxKvsValueBytes, CHIP_ERROR_INVALID_ARGUMENT); + JNIEnv * env = GetEnvForCurrentThread(); + ReturnErrorCodeIf(env == nullptr, CHIP_ERROR_INTERNAL); + char base64Buffer[kMaxKvsValueEncodedChars]; size_t length = chip::Base64Encode(static_cast(value), value_size, base64Buffer); base64Buffer[length] = 0; - UtfString utfKey(mEnv, key); - UtfString utfBase64Value(mEnv, base64Buffer); + UtfString utfKey(env, key); + UtfString utfBase64Value(env, base64Buffer); - mEnv->CallStaticVoidMethod(mKeyValueStoreManagerClass, mSetMethod, utfKey.jniValue(), utfBase64Value.jniValue()); + env->CallStaticVoidMethod(mKeyValueStoreManagerClass, mSetMethod, utfKey.jniValue(), utfBase64Value.jniValue()); - if (mEnv->ExceptionCheck()) + if (env->ExceptionCheck()) { ChipLogError(DeviceLayer, "Java exception in KVS::Delete"); - mEnv->ExceptionDescribe(); - mEnv->ExceptionClear(); + env->ExceptionDescribe(); + env->ExceptionClear(); return CHIP_JNI_ERROR_EXCEPTION_THROWN; } return CHIP_NO_ERROR; } -void KeyValueStoreManagerImpl::InitializeMethodForward(JNIEnv * env) +JNIEnv * KeyValueStoreManagerImpl::GetEnvForCurrentThread() +{ + if (mJvm == nullptr) + { + ChipLogError(DeviceLayer, "Missing Java VM in persistent storage"); + return nullptr; + } + + JNIEnv * env = nullptr; + + jint err = mJvm->AttachCurrentThread(&env, nullptr); + if (err != JNI_OK) + { + ChipLogError(DeviceLayer, "Failed to get JNIEnv for the current thread"); + return nullptr; + } + + return env; +} + +void KeyValueStoreManagerImpl::InitializeMethodForward(JavaVM * vm, JNIEnv * env) { - mEnv = env; + mJvm = vm; CHIP_ERROR err = GetClassRef(env, "chip/devicecontroller/KeyValueStoreManager", mKeyValueStoreManagerClass); if (err != CHIP_NO_ERROR) diff --git a/src/controller/java/AndroidKeyValueStoreManagerImpl.h b/src/controller/java/AndroidKeyValueStoreManagerImpl.h index ea471798952123..eeda32d5e4c9ba 100644 --- a/src/controller/java/AndroidKeyValueStoreManagerImpl.h +++ b/src/controller/java/AndroidKeyValueStoreManagerImpl.h @@ -31,16 +31,18 @@ class KeyValueStoreManagerImpl : public KeyValueStoreManager CHIP_ERROR _Delete(const char * key); CHIP_ERROR _Put(const char * key, const void * value, size_t value_size); - void InitializeMethodForward(JNIEnv * env); + void InitializeMethodForward(JavaVM * vm, JNIEnv * env); private: - JNIEnv * mEnv = nullptr; + JavaVM * mJvm = nullptr; jclass mKeyValueStoreManagerClass = nullptr; jmethodID mSetMethod = nullptr; jmethodID mGetMethod = nullptr; jmethodID mDeleteMethod = nullptr; + JNIEnv * GetEnvForCurrentThread(); + // ===== Members for internal use by the following friends. friend KeyValueStoreManager & KeyValueStoreMgr(); friend KeyValueStoreManagerImpl & KeyValueStoreMgrImpl(); diff --git a/src/controller/java/CHIPDeviceController-JNI.cpp b/src/controller/java/CHIPDeviceController-JNI.cpp index 73b33a9c43f98c..626d6d435e05d6 100644 --- a/src/controller/java/CHIPDeviceController-JNI.cpp +++ b/src/controller/java/CHIPDeviceController-JNI.cpp @@ -149,7 +149,7 @@ jint JNI_OnLoad(JavaVM * jvm, void * reserved) // Get a JNI environment object. sJVM->GetEnv((void **) &env, JNI_VERSION_1_6); - chip::DeviceLayer::PersistedStorage::KeyValueStoreMgrImpl().InitializeMethodForward(env); + chip::DeviceLayer::PersistedStorage::KeyValueStoreMgrImpl().InitializeMethodForward(sJVM, env); ChipLogProgress(Controller, "Loading Java class references.");