Fix android KVS implementation to work across threads (#6372)

project-chip · Jul 9, 2021 · 6d4647d · 6d4647d
1 parent 2242a7f
commit 6d4647d
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 24 deletions.
diff --git 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<const uint8_t *>(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
@@ -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
@@ -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.");