Proactively delete local JNI references after their referees are no
longer needed.
Failing to do so will eventually lead to running out of slots that track
all local references during native call.
Bug: 203214262
Test: atest ScriptExecutorUnitTest:ScriptExecutorTest
Test: atest ScriptExecutorUnitTest:JniUtilsTest
Change-Id: I55aa3d25d816de4732488112754da3b9882b5386
diff --git a/packages/ScriptExecutor/Android.bp b/packages/ScriptExecutor/Android.bp
index 643fb37..952a9b8 100644
--- a/packages/ScriptExecutor/Android.bp
+++ b/packages/ScriptExecutor/Android.bp
@@ -24,6 +24,7 @@
],
header_libs: [
"jni_headers",
+ "libnativehelper_header_only",
],
static_libs: [
diff --git a/packages/ScriptExecutor/src/BundleWrapper.cpp b/packages/ScriptExecutor/src/BundleWrapper.cpp
index 36c9196..88403b0 100644
--- a/packages/ScriptExecutor/src/BundleWrapper.cpp
+++ b/packages/ScriptExecutor/src/BundleWrapper.cpp
@@ -16,6 +16,8 @@
#include "BundleWrapper.h"
+#include "JniUtils.h"
+#include "nativehelper/scoped_local_ref.h"
namespace com {
namespace android {
@@ -25,31 +27,17 @@
using ::android::base::Error;
using ::android::base::Result;
-namespace {
-
-Result<jstring> TryCreateUTFString(JNIEnv* env, const char* string) {
- jstring utfString = env->NewStringUTF(string);
- if (env->ExceptionCheck()) {
- // NewStringUTF throws an exception if we run out of memory while creating a UTF string.
- return Error()
- << "NewStringUTF ran out of memory while converting a string provided by Lua.";
- }
- if (utfString == nullptr) {
- return Error()
- << "Failed to convert a Lua string into a modified UTF-8 string. Please verify "
- "that the string returned by Lua is in proper Modified UTF-8 format.";
- }
- return utfString;
-}
-
-} // namespace
-
BundleWrapper::BundleWrapper(JNIEnv* env) {
mJNIEnv = env;
- mBundleClass = static_cast<jclass>(
- mJNIEnv->NewGlobalRef(mJNIEnv->FindClass("android/os/PersistableBundle")));
+ ScopedLocalRef<jclass> localBundleClassRef(mJNIEnv,
+ mJNIEnv->FindClass("android/os/PersistableBundle"));
+ mBundleClass = static_cast<jclass>(mJNIEnv->NewGlobalRef(localBundleClassRef.get()));
+
jmethodID bundleConstructor = mJNIEnv->GetMethodID(mBundleClass, "<init>", "()V");
- mBundle = mJNIEnv->NewGlobalRef(mJNIEnv->NewObject(mBundleClass, bundleConstructor));
+ ScopedLocalRef<jobject> localBundleObjectRef(mJNIEnv,
+ mJNIEnv->NewObject(mBundleClass,
+ bundleConstructor));
+ mBundle = mJNIEnv->NewGlobalRef(localBundleObjectRef.get());
}
BundleWrapper::~BundleWrapper() {
@@ -63,44 +51,40 @@
}
Result<void> BundleWrapper::putBoolean(const char* key, bool value) {
- auto keyStringResult = TryCreateUTFString(mJNIEnv, key);
- if (!keyStringResult.ok()) {
- return Error() << "Failed to create a string for key=" << key << ". "
- << keyStringResult.error();
+ ScopedLocalRef<jstring> keyStringRef(mJNIEnv, mJNIEnv->NewStringUTF(key));
+ if (keyStringRef == nullptr) {
+ return Error() << "Failed to create a string for a key=" << key << " due to OOM error";
}
// TODO(b/188832769): consider caching the references.
jmethodID putBooleanMethod =
mJNIEnv->GetMethodID(mBundleClass, "putBoolean", "(Ljava/lang/String;Z)V");
- mJNIEnv->CallVoidMethod(mBundle, putBooleanMethod, keyStringResult.value(),
+ mJNIEnv->CallVoidMethod(mBundle, putBooleanMethod, keyStringRef.get(),
static_cast<jboolean>(value));
return {}; // ok result
}
Result<void> BundleWrapper::putLong(const char* key, int64_t value) {
- auto keyStringResult = TryCreateUTFString(mJNIEnv, key);
- if (!keyStringResult.ok()) {
- return Error() << "Failed to create a string for key=" << key << ". "
- << keyStringResult.error();
+ ScopedLocalRef<jstring> keyStringRef(mJNIEnv, mJNIEnv->NewStringUTF(key));
+ if (keyStringRef == nullptr) {
+ return Error() << "Failed to create a string for a key=" << key << " due to OOM error";
}
jmethodID putLongMethod =
mJNIEnv->GetMethodID(mBundleClass, "putLong", "(Ljava/lang/String;J)V");
- mJNIEnv->CallVoidMethod(mBundle, putLongMethod, keyStringResult.value(),
- static_cast<jlong>(value));
+ mJNIEnv->CallVoidMethod(mBundle, putLongMethod, keyStringRef.get(), static_cast<jlong>(value));
return {}; // ok result
}
Result<void> BundleWrapper::putDouble(const char* key, double value) {
- auto keyStringResult = TryCreateUTFString(mJNIEnv, key);
- if (!keyStringResult.ok()) {
- return Error() << "Failed to create a string for key=" << key << ". "
- << keyStringResult.error();
+ ScopedLocalRef<jstring> keyStringRef(mJNIEnv, mJNIEnv->NewStringUTF(key));
+ if (keyStringRef == nullptr) {
+ return Error() << "Failed to create a string for a key=" << key << " due to OOM error";
}
jmethodID putDoubleMethod =
mJNIEnv->GetMethodID(mBundleClass, "putDouble", "(Ljava/lang/String;D)V");
- mJNIEnv->CallVoidMethod(mBundle, putDoubleMethod, keyStringResult.value(),
+ mJNIEnv->CallVoidMethod(mBundle, putDoubleMethod, keyStringRef.get(),
static_cast<jdouble>(value));
return {}; // ok result
}
@@ -108,64 +92,59 @@
Result<void> BundleWrapper::putString(const char* key, const char* value) {
jmethodID putStringMethod = mJNIEnv->GetMethodID(mBundleClass, "putString",
"(Ljava/lang/String;Ljava/lang/String;)V");
- // TODO(b/201008922): Handle a case when NewStringUTF returns nullptr (fails
- // to create a string).
- auto keyStringResult = TryCreateUTFString(mJNIEnv, key);
- if (!keyStringResult.ok()) {
- return Error() << "Failed to create a string for key=" << key << ". "
- << keyStringResult.error();
- }
- auto valueStringResult = TryCreateUTFString(mJNIEnv, value);
- if (!valueStringResult.ok()) {
- return Error() << "Failed to create a string for value=" << value << ". "
- << valueStringResult.error();
+ ScopedLocalRef<jstring> keyStringRef(mJNIEnv, mJNIEnv->NewStringUTF(key));
+ if (keyStringRef == nullptr) {
+ return Error() << "Failed to create a string for a key=" << key << " due to OOM error";
}
- mJNIEnv->CallVoidMethod(mBundle, putStringMethod, keyStringResult.value(),
- valueStringResult.value());
+ ScopedLocalRef<jstring> valueStringRef(mJNIEnv, mJNIEnv->NewStringUTF(value));
+ if (valueStringRef == nullptr) {
+ return Error() << "Failed to create a string for the provided value due to OOM error";
+ }
+
+ mJNIEnv->CallVoidMethod(mBundle, putStringMethod, keyStringRef.get(), valueStringRef.get());
return {}; // ok result
}
Result<void> BundleWrapper::putLongArray(const char* key, const std::vector<int64_t>& value) {
- auto keyStringResult = TryCreateUTFString(mJNIEnv, key);
- if (!keyStringResult.ok()) {
- return Error() << "Failed to create a string for key=" << key << ". "
- << keyStringResult.error();
+ ScopedLocalRef<jstring> keyStringRef(mJNIEnv, mJNIEnv->NewStringUTF(key));
+ if (keyStringRef == nullptr) {
+ return Error() << "Failed to create a string for a key=" << key << " due to OOM error";
}
jmethodID putLongArrayMethod =
mJNIEnv->GetMethodID(mBundleClass, "putLongArray", "(Ljava/lang/String;[J)V");
- jlongArray array = mJNIEnv->NewLongArray(value.size());
- mJNIEnv->SetLongArrayRegion(array, 0, value.size(), &value[0]);
- mJNIEnv->CallVoidMethod(mBundle, putLongArrayMethod, keyStringResult.value(), array);
+ ScopedLocalRef<jlongArray> arrayRef(mJNIEnv, mJNIEnv->NewLongArray(value.size()));
+ mJNIEnv->SetLongArrayRegion(arrayRef.get(), 0, value.size(), &value[0]);
+ mJNIEnv->CallVoidMethod(mBundle, putLongArrayMethod, keyStringRef.get(), arrayRef.get());
return {}; // ok result
}
Result<void> BundleWrapper::putStringArray(const char* key, const std::vector<std::string>& value) {
- auto keyStringResult = TryCreateUTFString(mJNIEnv, key);
- if (!keyStringResult.ok()) {
- return Error() << "Failed to create a string for key=" << key << ". "
- << keyStringResult.error();
+ ScopedLocalRef<jstring> keyStringRef(mJNIEnv, mJNIEnv->NewStringUTF(key));
+ if (keyStringRef == nullptr) {
+ return Error() << "Failed to create a string for a key=" << key << " due to OOM error";
}
jmethodID putStringArrayMethod =
mJNIEnv->GetMethodID(mBundleClass, "putStringArray",
"(Ljava/lang/String;[Ljava/lang/String;)V");
- jobjectArray array =
- mJNIEnv->NewObjectArray(value.size(), mJNIEnv->FindClass("java/lang/String"), nullptr);
- // TODO(b/201008922): Handle a case when NewStringUTF returns nullptr (fails
- // to create a string).
+ ScopedLocalRef<jclass> stringClassRef(mJNIEnv, mJNIEnv->FindClass("java/lang/String"));
+ ScopedLocalRef<jobjectArray> arrayRef(mJNIEnv,
+ mJNIEnv->NewObjectArray(value.size(),
+ mJNIEnv->FindClass(
+ "java/lang/String"),
+ nullptr));
for (int i = 0; i < value.size(); i++) {
- auto valueStringResult = TryCreateUTFString(mJNIEnv, value[i].c_str());
- if (!valueStringResult.ok()) {
- return Error() << "Failed to create a string for value=" << value[i].c_str() << ". "
- << valueStringResult.error();
+ ScopedLocalRef<jstring> valueStringRef(mJNIEnv, mJNIEnv->NewStringUTF(value[i].c_str()));
+ if (valueStringRef == nullptr) {
+ return Error() << "Failed to create a string for provided value due to OOM error";
}
- mJNIEnv->SetObjectArrayElement(array, i, valueStringResult.value());
+ mJNIEnv->SetObjectArrayElement(arrayRef.get(), i, valueStringRef.get());
}
- mJNIEnv->CallVoidMethod(mBundle, putStringArrayMethod, keyStringResult.value(), array);
+ mJNIEnv->CallVoidMethod(mBundle, putStringArrayMethod, keyStringRef.get(), arrayRef.get());
return {}; // ok result
}
diff --git a/packages/ScriptExecutor/src/JniUtils.cpp b/packages/ScriptExecutor/src/JniUtils.cpp
index adc8d84..0d3f472 100644
--- a/packages/ScriptExecutor/src/JniUtils.cpp
+++ b/packages/ScriptExecutor/src/JniUtils.cpp
@@ -16,13 +16,15 @@
#include "JniUtils.h"
+#include "nativehelper/scoped_local_ref.h"
+
namespace com {
namespace android {
namespace car {
namespace scriptexecutor {
-void pushBundleToLuaTable(JNIEnv* env, LuaEngine* luaEngine, jobject bundle) {
- lua_newtable(luaEngine->getLuaState());
+void pushBundleToLuaTable(JNIEnv* env, lua_State* lua, jobject bundle) {
+ lua_newtable(lua);
// null bundle object is allowed. We will treat it as an empty table.
if (bundle == nullptr) {
return;
@@ -30,127 +32,134 @@
// TODO(b/188832769): Consider caching some of these JNI references for
// performance reasons.
- jclass persistableBundleClass = env->FindClass("android/os/PersistableBundle");
+ ScopedLocalRef<jclass> persistableBundleClass(env,
+ env->FindClass("android/os/PersistableBundle"));
jmethodID getKeySetMethod =
- env->GetMethodID(persistableBundleClass, "keySet", "()Ljava/util/Set;");
- jobject keys = env->CallObjectMethod(bundle, getKeySetMethod);
- jclass setClass = env->FindClass("java/util/Set");
- jmethodID iteratorMethod = env->GetMethodID(setClass, "iterator", "()Ljava/util/Iterator;");
- jobject keySetIteratorObject = env->CallObjectMethod(keys, iteratorMethod);
+ env->GetMethodID(persistableBundleClass.get(), "keySet", "()Ljava/util/Set;");
+ ScopedLocalRef<jobject> keys(env, env->CallObjectMethod(bundle, getKeySetMethod));
+ ScopedLocalRef<jclass> setClass(env, env->FindClass("java/util/Set"));
+ jmethodID iteratorMethod =
+ env->GetMethodID(setClass.get(), "iterator", "()Ljava/util/Iterator;");
+ ScopedLocalRef<jobject> keySetIteratorObject(env,
+ env->CallObjectMethod(keys.get(), iteratorMethod));
- jclass iteratorClass = env->FindClass("java/util/Iterator");
- jmethodID hasNextMethod = env->GetMethodID(iteratorClass, "hasNext", "()Z");
- jmethodID nextMethod = env->GetMethodID(iteratorClass, "next", "()Ljava/lang/Object;");
+ ScopedLocalRef<jclass> iteratorClass(env, env->FindClass("java/util/Iterator"));
+ jmethodID hasNextMethod = env->GetMethodID(iteratorClass.get(), "hasNext", "()Z");
+ jmethodID nextMethod = env->GetMethodID(iteratorClass.get(), "next", "()Ljava/lang/Object;");
- jclass booleanClass = env->FindClass("java/lang/Boolean");
- jclass integerClass = env->FindClass("java/lang/Integer");
- jclass longClass = env->FindClass("java/lang/Long");
- jclass numberClass = env->FindClass("java/lang/Number");
- jclass stringClass = env->FindClass("java/lang/String");
- jclass intArrayClass = env->FindClass("[I");
- jclass longArrayClass = env->FindClass("[J");
- jclass stringArrayClass = env->FindClass("[Ljava/lang/String;");
+ ScopedLocalRef<jclass> booleanClass(env, env->FindClass("java/lang/Boolean"));
+ ScopedLocalRef<jclass> integerClass(env, env->FindClass("java/lang/Integer"));
+ ScopedLocalRef<jclass> longClass(env, env->FindClass("java/lang/Long"));
+ ScopedLocalRef<jclass> numberClass(env, env->FindClass("java/lang/Number"));
+ ScopedLocalRef<jclass> stringClass(env, env->FindClass("java/lang/String"));
+ ScopedLocalRef<jclass> intArrayClass(env, env->FindClass("[I"));
+ ScopedLocalRef<jclass> longArrayClass(env, env->FindClass("[J"));
+ ScopedLocalRef<jclass> stringArrayClass(env, env->FindClass("[Ljava/lang/String;"));
// TODO(b/188816922): Handle more types such as float and integer arrays,
// and perhaps nested Bundles.
- jmethodID getMethod = env->GetMethodID(persistableBundleClass, "get",
+ jmethodID getMethod = env->GetMethodID(persistableBundleClass.get(), "get",
"(Ljava/lang/String;)Ljava/lang/Object;");
// Iterate over key set of the bundle one key at a time.
- while (env->CallBooleanMethod(keySetIteratorObject, hasNextMethod)) {
+ while (env->CallBooleanMethod(keySetIteratorObject.get(), hasNextMethod)) {
// Read the value object that corresponds to this key.
- jstring key = (jstring)env->CallObjectMethod(keySetIteratorObject, nextMethod);
- jobject value = env->CallObjectMethod(bundle, getMethod, key);
+ ScopedLocalRef<jstring> key(env,
+ (jstring)env->CallObjectMethod(keySetIteratorObject.get(),
+ nextMethod));
+ ScopedLocalRef<jobject> value(env, env->CallObjectMethod(bundle, getMethod, key.get()));
// Get the value of the type, extract it accordingly from the bundle and
// push the extracted value and the key to the Lua table.
- if (env->IsInstanceOf(value, booleanClass)) {
- jmethodID boolMethod = env->GetMethodID(booleanClass, "booleanValue", "()Z");
- bool boolValue = static_cast<bool>(env->CallBooleanMethod(value, boolMethod));
- lua_pushboolean(luaEngine->getLuaState(), boolValue);
- } else if (env->IsInstanceOf(value, integerClass)) {
- jmethodID intMethod = env->GetMethodID(integerClass, "intValue", "()I");
- lua_pushinteger(luaEngine->getLuaState(), env->CallIntMethod(value, intMethod));
- } else if (env->IsInstanceOf(value, longClass)) {
- jmethodID longMethod = env->GetMethodID(longClass, "longValue", "()J");
- lua_pushinteger(luaEngine->getLuaState(), env->CallLongMethod(value, longMethod));
- } else if (env->IsInstanceOf(value, numberClass)) {
+ if (env->IsInstanceOf(value.get(), booleanClass.get())) {
+ jmethodID boolMethod = env->GetMethodID(booleanClass.get(), "booleanValue", "()Z");
+ bool boolValue = static_cast<bool>(env->CallBooleanMethod(value.get(), boolMethod));
+ lua_pushboolean(lua, boolValue);
+ } else if (env->IsInstanceOf(value.get(), integerClass.get())) {
+ jmethodID intMethod = env->GetMethodID(integerClass.get(), "intValue", "()I");
+ lua_pushinteger(lua, env->CallIntMethod(value.get(), intMethod));
+ } else if (env->IsInstanceOf(value.get(), longClass.get())) {
+ jmethodID longMethod = env->GetMethodID(longClass.get(), "longValue", "()J");
+ lua_pushinteger(lua, env->CallLongMethod(value.get(), longMethod));
+ } else if (env->IsInstanceOf(value.get(), numberClass.get())) {
// Condense other numeric types using one class. Because lua supports only
// integer or double, and we handled integer in previous if clause.
- jmethodID numberMethod = env->GetMethodID(numberClass, "doubleValue", "()D");
+ jmethodID numberMethod = env->GetMethodID(numberClass.get(), "doubleValue", "()D");
/* Pushes a double onto the stack */
- lua_pushnumber(luaEngine->getLuaState(), env->CallDoubleMethod(value, numberMethod));
- } else if (env->IsInstanceOf(value, stringClass)) {
+ lua_pushnumber(lua, env->CallDoubleMethod(value.get(), numberMethod));
+ } else if (env->IsInstanceOf(value.get(), stringClass.get())) {
// Produces a string in Modified UTF-8 encoding. Any null character
// inside the original string is converted into two-byte encoding.
// This way we can directly use the output of GetStringUTFChars in C API that
// expects a null-terminated string.
const char* rawStringValue =
- env->GetStringUTFChars(static_cast<jstring>(value), nullptr);
- lua_pushstring(luaEngine->getLuaState(), rawStringValue);
- env->ReleaseStringUTFChars(static_cast<jstring>(value), rawStringValue);
- } else if (env->IsInstanceOf(value, intArrayClass)) {
- jintArray intArray = static_cast<jintArray>(value);
+ env->GetStringUTFChars(static_cast<jstring>(value.get()), nullptr);
+ lua_pushstring(lua, rawStringValue);
+ env->ReleaseStringUTFChars(static_cast<jstring>(value.get()), rawStringValue);
+ } else if (env->IsInstanceOf(value.get(), intArrayClass.get())) {
+ jintArray intArray = static_cast<jintArray>(value.get());
const auto kLength = env->GetArrayLength(intArray);
// Arrays are represented as a table of sequential elements in Lua.
// We are creating a nested table to represent this array. We specify number of elements
// in the Java array to preallocate memory accordingly.
- lua_createtable(luaEngine->getLuaState(), kLength, 0);
+ lua_createtable(lua, kLength, 0);
jint* rawIntArray = env->GetIntArrayElements(intArray, nullptr);
// Fills in the table at stack idx -2 with key value pairs, where key is a
// Lua index and value is an integer from the byte array at that index
for (int i = 0; i < kLength; i++) {
// Stack at index -1 is rawIntArray[i] after this push.
- lua_pushinteger(luaEngine->getLuaState(), rawIntArray[i]);
- lua_rawseti(luaEngine->getLuaState(), /* idx= */ -2,
+ lua_pushinteger(lua, rawIntArray[i]);
+ lua_rawseti(lua, /* idx= */ -2,
i + 1); // lua index starts from 1
}
// JNI_ABORT is used because we do not need to copy back elements.
env->ReleaseIntArrayElements(intArray, rawIntArray, JNI_ABORT);
- } else if (env->IsInstanceOf(value, longArrayClass)) {
- jlongArray longArray = static_cast<jlongArray>(value);
+ } else if (env->IsInstanceOf(value.get(), longArrayClass.get())) {
+ jlongArray longArray = static_cast<jlongArray>(value.get());
const auto kLength = env->GetArrayLength(longArray);
// Arrays are represented as a table of sequential elements in Lua.
// We are creating a nested table to represent this array. We specify number of elements
// in the Java array to preallocate memory accordingly.
- lua_createtable(luaEngine->getLuaState(), kLength, 0);
+ lua_createtable(lua, kLength, 0);
jlong* rawLongArray = env->GetLongArrayElements(longArray, nullptr);
// Fills in the table at stack idx -2 with key value pairs, where key is a
// Lua index and value is an integer from the byte array at that index
for (int i = 0; i < kLength; i++) {
- lua_pushinteger(luaEngine->getLuaState(), rawLongArray[i]);
- lua_rawseti(luaEngine->getLuaState(), /* idx= */ -2,
+ lua_pushinteger(lua, rawLongArray[i]);
+ lua_rawseti(lua, /* idx= */ -2,
i + 1); // lua index starts from 1
}
// JNI_ABORT is used because we do not need to copy back elements.
env->ReleaseLongArrayElements(longArray, rawLongArray, JNI_ABORT);
- } else if (env->IsInstanceOf(value, stringArrayClass)) {
- jobjectArray stringArray = static_cast<jobjectArray>(value);
+ } else if (env->IsInstanceOf(value.get(), stringArrayClass.get())) {
+ jobjectArray stringArray = static_cast<jobjectArray>(value.get());
const auto kLength = env->GetArrayLength(stringArray);
// Arrays are represented as a table of sequential elements in Lua.
// We are creating a nested table to represent this array. We specify number of elements
// in the Java array to preallocate memory accordingly.
- lua_createtable(luaEngine->getLuaState(), kLength, 0);
+ lua_createtable(lua, kLength, 0);
// Fills in the table at stack idx -2 with key value pairs, where key is a Lua index and
// value is an string value extracted from the object array at that index
for (int i = 0; i < kLength; i++) {
- jstring element = static_cast<jstring>(env->GetObjectArrayElement(stringArray, i));
+ ScopedLocalRef<jobject> localStringRef(env,
+ env->GetObjectArrayElement(stringArray, i));
+ jstring element = static_cast<jstring>(localStringRef.get());
const char* rawStringValue = env->GetStringUTFChars(element, nullptr);
- lua_pushstring(luaEngine->getLuaState(), rawStringValue);
+ lua_pushstring(lua, rawStringValue);
env->ReleaseStringUTFChars(element, rawStringValue);
// lua index starts from 1
- lua_rawseti(luaEngine->getLuaState(), /* idx= */ -2, i + 1);
+ lua_rawseti(lua, /* idx= */ -2, i + 1);
}
} else {
// Other types are not implemented yet, skipping.
continue;
}
- const char* rawKey = env->GetStringUTFChars(key, nullptr);
+ const char* rawKey = env->GetStringUTFChars(key.get(), nullptr);
// table[rawKey] = value, where value is on top of the stack,
// and the table is the next element in the stack.
- lua_setfield(luaEngine->getLuaState(), /* idx= */ -2, rawKey);
- env->ReleaseStringUTFChars(key, rawKey);
+ lua_setfield(lua, /* idx= */ -2, rawKey);
+ env->ReleaseStringUTFChars(key.get(), rawKey);
}
}
diff --git a/packages/ScriptExecutor/src/JniUtils.h b/packages/ScriptExecutor/src/JniUtils.h
index 57aeb9f..542a91a 100644
--- a/packages/ScriptExecutor/src/JniUtils.h
+++ b/packages/ScriptExecutor/src/JniUtils.h
@@ -16,9 +16,14 @@
#ifndef PACKAGES_SCRIPTEXECUTOR_SRC_JNIUTILS_H_
#define PACKAGES_SCRIPTEXECUTOR_SRC_JNIUTILS_H_
-#include "LuaEngine.h"
#include "jni.h"
+extern "C" {
+#include "lua.h"
+}
+
+#include <android-base/result.h>
+
namespace com {
namespace android {
namespace car {
@@ -29,7 +34,7 @@
// converted to the corresponding key-value pairs of the Lua table as long as
// the Bundle value types are supported. At this point, we support boolean,
// integer, double and String types in Java.
-void pushBundleToLuaTable(JNIEnv* env, LuaEngine* luaEngine, jobject bundle);
+void pushBundleToLuaTable(JNIEnv* env, lua_State* lua, jobject bundle);
} // namespace scriptexecutor
} // namespace car
diff --git a/packages/ScriptExecutor/src/ScriptExecutorJni.cpp b/packages/ScriptExecutor/src/ScriptExecutorJni.cpp
index cab8ece..d8ddd55 100644
--- a/packages/ScriptExecutor/src/ScriptExecutorJni.cpp
+++ b/packages/ScriptExecutor/src/ScriptExecutorJni.cpp
@@ -83,6 +83,10 @@
LuaEngine* engine = reinterpret_cast<LuaEngine*>(static_cast<intptr_t>(luaEnginePtr));
LuaEngine::resetListener(new ScriptExecutorListener(env, listener));
+ // Objects passed to native methods are local JNI references.
+ // ScriptExecutorListener constructor above converts local "listener" reference to global
+ // reference. Delete local "listener" reference here because it is no longer needed.
+ env->DeleteLocalRef(listener);
// Load and parse the script
const char* scriptStr = env->GetStringUTFChars(scriptBody, nullptr);
@@ -103,11 +107,11 @@
}
// Unpack bundle in publishedData, convert to Lua table and push it to Lua stack.
- pushBundleToLuaTable(env, engine, publishedData);
+ pushBundleToLuaTable(env, engine->getLuaState(), publishedData);
// Unpack bundle in savedState, convert to Lua table and push it to Lua
// stack.
- pushBundleToLuaTable(env, engine, savedState);
+ pushBundleToLuaTable(env, engine->getLuaState(), savedState);
// Execute the function. This will block until complete or error.
engine->run();
diff --git a/packages/ScriptExecutor/src/ScriptExecutorListener.cpp b/packages/ScriptExecutor/src/ScriptExecutorListener.cpp
index fd6677d..b9cd7c8 100644
--- a/packages/ScriptExecutor/src/ScriptExecutorListener.cpp
+++ b/packages/ScriptExecutor/src/ScriptExecutorListener.cpp
@@ -16,6 +16,9 @@
#include "ScriptExecutorListener.h"
+#include "JniUtils.h"
+#include "nativehelper/scoped_local_ref.h"
+
#include <android-base/logging.h>
namespace com {
@@ -37,16 +40,16 @@
void ScriptExecutorListener::onSuccess(jobject bundle) {
JNIEnv* env = getCurrentJNIEnv();
- jclass listenerClass = env->GetObjectClass(mScriptExecutorListener);
- jmethodID onSuccessMethod =
- env->GetMethodID(listenerClass, "onSuccess", "(Landroid/os/PersistableBundle;)V");
+ ScopedLocalRef<jclass> listenerClassRef(env, env->GetObjectClass(mScriptExecutorListener));
+ jmethodID onSuccessMethod = env->GetMethodID(listenerClassRef.get(), "onSuccess",
+ "(Landroid/os/PersistableBundle;)V");
env->CallVoidMethod(mScriptExecutorListener, onSuccessMethod, bundle);
}
void ScriptExecutorListener::onScriptFinished(jobject bundle) {
JNIEnv* env = getCurrentJNIEnv();
- jclass listenerClass = env->GetObjectClass(mScriptExecutorListener);
- jmethodID onScriptFinished = env->GetMethodID(listenerClass, "onScriptFinished",
+ ScopedLocalRef<jclass> listenerClassRef(env, env->GetObjectClass(mScriptExecutorListener));
+ jmethodID onScriptFinished = env->GetMethodID(listenerClassRef.get(), "onScriptFinished",
"(Landroid/os/PersistableBundle;)V");
env->CallVoidMethod(mScriptExecutorListener, onScriptFinished, bundle);
}
@@ -54,12 +57,24 @@
void ScriptExecutorListener::onError(const ErrorType errorType, const char* message,
const char* stackTrace) {
JNIEnv* env = getCurrentJNIEnv();
- jclass listenerClass = env->GetObjectClass(mScriptExecutorListener);
- jmethodID onErrorMethod =
- env->GetMethodID(listenerClass, "onError", "(ILjava/lang/String;Ljava/lang/String;)V");
+ ScopedLocalRef<jclass> listenerClassRef(env, env->GetObjectClass(mScriptExecutorListener));
+ jmethodID onErrorMethod = env->GetMethodID(listenerClassRef.get(), "onError",
+ "(ILjava/lang/String;Ljava/lang/String;)V");
+
+ ScopedLocalRef<jstring> messageStringRef(env, env->NewStringUTF(message));
+ if (messageStringRef == nullptr) {
+ LOG(ERROR) << "Failed to create a Java string for provided error message due to OOM error";
+ return;
+ }
+
+ ScopedLocalRef<jstring> stackTraceRef(env, env->NewStringUTF(stackTrace));
+ if (stackTraceRef == nullptr) {
+ LOG(ERROR) << "Failed to create a Java string for stack trace due to OOM error";
+ return;
+ }
env->CallVoidMethod(mScriptExecutorListener, onErrorMethod, static_cast<int>(errorType),
- env->NewStringUTF(message), env->NewStringUTF(stackTrace));
+ messageStringRef.get(), stackTraceRef.get());
}
JNIEnv* ScriptExecutorListener::getCurrentJNIEnv() {
diff --git a/packages/ScriptExecutor/tests/unit/src/com/android/car/scriptexecutor/JniUtilsTestHelper.cpp b/packages/ScriptExecutor/tests/unit/src/com/android/car/scriptexecutor/JniUtilsTestHelper.cpp
index efc34d3..a205b32 100644
--- a/packages/ScriptExecutor/tests/unit/src/com/android/car/scriptexecutor/JniUtilsTestHelper.cpp
+++ b/packages/ScriptExecutor/tests/unit/src/com/android/car/scriptexecutor/JniUtilsTestHelper.cpp
@@ -83,8 +83,8 @@
JNIEXPORT void JNICALL
Java_com_android_car_scriptexecutor_JniUtilsTest_nativePushBundleToLuaTableCaller(
JNIEnv* env, jobject object, jlong luaEnginePtr, jobject bundle) {
- pushBundleToLuaTable(env, reinterpret_cast<LuaEngine*>(static_cast<intptr_t>(luaEnginePtr)),
- bundle);
+ LuaEngine* engine = reinterpret_cast<LuaEngine*>(static_cast<intptr_t>(luaEnginePtr));
+ pushBundleToLuaTable(env, engine->getLuaState(), bundle);
}
JNIEXPORT jint JNICALL Java_com_android_car_scriptexecutor_JniUtilsTest_nativeGetObjectSize(
diff --git a/packages/ScriptExecutor/tests/unit/src/com/android/car/scriptexecutor/ScriptExecutorTest.java b/packages/ScriptExecutor/tests/unit/src/com/android/car/scriptexecutor/ScriptExecutorTest.java
index 5decaa3..1a4028a 100644
--- a/packages/ScriptExecutor/tests/unit/src/com/android/car/scriptexecutor/ScriptExecutorTest.java
+++ b/packages/ScriptExecutor/tests/unit/src/com/android/car/scriptexecutor/ScriptExecutorTest.java
@@ -524,6 +524,23 @@
}
@Test
+ public void invokeScript_emptyStringValueIsValidValue() throws RemoteException {
+ // Verify that an empty string value is a valid value to be returned from a script.
+ String returnFinalResultScript =
+ "function empty_string(data, state)\n"
+ + " result = {data = \"\"}\n"
+ + " on_script_finished(result)\n"
+ + "end\n";
+
+ runScriptAndWaitForResponse(returnFinalResultScript, "empty_string", mPublishedData,
+ new PersistableBundle());
+
+ // Expect to get back a bundle with a single key-value pair {"data": ""}
+ assertThat(mFakeScriptExecutorListener.mFinalResult.size()).isEqualTo(1);
+ assertThat(mFakeScriptExecutorListener.mFinalResult.getString("data")).isEqualTo("");
+ }
+
+ @Test
public void invokeScript_allPrimitiveSupportedTypesForReturningFinalResult()
throws RemoteException {
// Here we verify that all supported primitive types are present in the returned final