Skip to content

Commit

Permalink
[Android] Remove JniClass and fix multiple local/global reference lea…
Browse files Browse the repository at this point in the history 
…ks in JNI (#31785)

* [Android]Fix multiple local/global reference leaks in JNI

* add generated files

* fix ci
  • Loading branch information
@yunhanw-google @pull
yunhanw-google authored and pull[bot] committed Feb 28, 2024
1 parent fede02f commit 1023378
Show file tree
Hide file tree
Showing 21 changed files with 330 additions and 407 deletions.
22 changes: 13 additions & 9 deletions examples/tv-casting-app/android/App/app/src/main/jni/cpp/ConversionUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ CHIP_ERROR convertJAppParametersToCppAppParams(jobject appParameters, AppParams

jclass jAppParametersClass;
ReturnErrorOnFailure(
chip::JniReferences::GetInstance().GetClassRef(env, "com/chip/casting/AppParameters", jAppParametersClass));
chip::JniReferences::GetInstance().GetLocalClassRef(env, "com/chip/casting/AppParameters", jAppParametersClass));

jmethodID getRotatingDeviceIdUniqueIdMethod = env->GetMethodID(jAppParametersClass, "getRotatingDeviceIdUniqueId", "()[B");
if (getRotatingDeviceIdUniqueIdMethod == nullptr)
Expand Down Expand Up @@ -67,7 +67,7 @@ CHIP_ERROR convertJContentAppToTargetEndpointInfo(jobject contentApp, TargetEndp
JNIEnv * env = chip::JniReferences::GetInstance().GetEnvForCurrentThread();

jclass jContentAppClass;
ReturnErrorOnFailure(chip::JniReferences::GetInstance().GetClassRef(env, "com/chip/casting/ContentApp", jContentAppClass));
ReturnErrorOnFailure(chip::JniReferences::GetInstance().GetLocalClassRef(env, "com/chip/casting/ContentApp", jContentAppClass));

jfieldID jEndpointIdField = env->GetFieldID(jContentAppClass, "endpointId", "S");
jshort jEndpointId = env->GetShortField(contentApp, jEndpointIdField);
Expand Down Expand Up @@ -104,7 +104,8 @@ CHIP_ERROR convertTargetEndpointInfoToJContentApp(TargetEndpointInfo * targetEnd
JNIEnv * env = chip::JniReferences::GetInstance().GetEnvForCurrentThread();

jclass jContentAppClass;
ReturnErrorOnFailure(chip::JniReferences::GetInstance().GetClassRef(env, "com/chip/casting/ContentApp", jContentAppClass));
ReturnErrorOnFailure(
chip::JniReferences::GetInstance().GetLocalClassRef(env, "com/chip/casting/ContentApp", jContentAppClass));
jmethodID jContentAppConstructor = env->GetMethodID(jContentAppClass, "<init>", "(SLjava/util/List;)V");
chip::ClusterId * clusters = targetEndpointInfo->GetClusters();
jobject jClustersArrayList = nullptr;
Expand Down Expand Up @@ -132,7 +133,8 @@ CHIP_ERROR convertJVideoPlayerToTargetVideoPlayerInfo(jobject videoPlayer, Targe
JNIEnv * env = chip::JniReferences::GetInstance().GetEnvForCurrentThread();

jclass jVideoPlayerClass;
ReturnErrorOnFailure(chip::JniReferences::GetInstance().GetClassRef(env, "com/chip/casting/VideoPlayer", jVideoPlayerClass));
ReturnErrorOnFailure(
chip::JniReferences::GetInstance().GetLocalClassRef(env, "com/chip/casting/VideoPlayer", jVideoPlayerClass));

jfieldID jNodeIdField = env->GetFieldID(jVideoPlayerClass, "nodeId", "J");
chip::NodeId nodeId = static_cast<chip::NodeId>(env->GetLongField(videoPlayer, jNodeIdField));
Expand Down Expand Up @@ -206,7 +208,8 @@ CHIP_ERROR convertJVideoPlayerToTargetVideoPlayerInfo(jobject videoPlayer, Targe
jobject jContentApp = env->CallObjectMethod(jIterator, jNextMid);

jclass jContentAppClass;
ReturnErrorOnFailure(chip::JniReferences::GetInstance().GetClassRef(env, "com/chip/casting/ContentApp", jContentAppClass));
ReturnErrorOnFailure(
chip::JniReferences::GetInstance().GetLocalClassRef(env, "com/chip/casting/ContentApp", jContentAppClass));
jfieldID jEndpointIdField = env->GetFieldID(jContentAppClass, "endpointId", "S");
chip::EndpointId endpointId = static_cast<chip::EndpointId>(env->GetShortField(jContentApp, jEndpointIdField));
TargetEndpointInfo * endpoint = outTargetVideoPlayerInfo.GetOrAddEndpoint(endpointId);
Expand All @@ -226,7 +229,7 @@ CHIP_ERROR convertTargetVideoPlayerInfoToJVideoPlayer(TargetVideoPlayerInfo * ta

jclass jVideoPlayerClass;
ReturnErrorOnFailure(
chip::JniReferences::GetInstance().GetClassRef(env, "com/chip/casting/VideoPlayer", jVideoPlayerClass));
chip::JniReferences::GetInstance().GetLocalClassRef(env, "com/chip/casting/VideoPlayer", jVideoPlayerClass));
jmethodID jVideoPlayerConstructor = env->GetMethodID(jVideoPlayerClass, "<init>",
"(JBLjava/lang/String;IIILjava/util/List;ILjava/util/List;Ljava/lang/"
"String;Ljava/lang/String;IJLjava/lang/String;ZZ)V");
Expand Down Expand Up @@ -277,7 +280,8 @@ CHIP_ERROR convertTargetVideoPlayerInfoToJVideoPlayer(TargetVideoPlayerInfo * ta
jstring jIPAddressStr = env->NewStringUTF(addrCString);

jclass jIPAddressClass;
ReturnErrorOnFailure(chip::JniReferences::GetInstance().GetClassRef(env, "java/net/InetAddress", jIPAddressClass));
ReturnErrorOnFailure(
chip::JniReferences::GetInstance().GetLocalClassRef(env, "java/net/InetAddress", jIPAddressClass));
jmethodID jGetByNameMid =
env->GetStaticMethodID(jIPAddressClass, "getByName", "(Ljava/lang/String;)Ljava/net/InetAddress;");
jobject jIPAddress = env->CallStaticObjectMethod(jIPAddressClass, jGetByNameMid, jIPAddressStr);
Expand Down Expand Up @@ -306,7 +310,7 @@ CHIP_ERROR convertJDiscoveredNodeDataToCppDiscoveredNodeData(jobject jDiscovered

jclass jDiscoveredNodeDataClass;
ReturnErrorOnFailure(
chip::JniReferences::GetInstance().GetClassRef(env, "com/chip/casting/DiscoveredNodeData", jDiscoveredNodeDataClass));
chip::JniReferences::GetInstance().GetLocalClassRef(env, "com/chip/casting/DiscoveredNodeData", jDiscoveredNodeDataClass));

jfieldID getHostNameField = env->GetFieldID(jDiscoveredNodeDataClass, "hostName", "Ljava/lang/String;");
jstring jHostName = static_cast<jstring>(env->GetObjectField(jDiscoveredNodeData, getHostNameField));
Expand Down Expand Up @@ -392,7 +396,7 @@ CHIP_ERROR convertJDiscoveredNodeDataToCppDiscoveredNodeData(jobject jDiscovered
{
jobject jIPAddress = env->CallObjectMethod(jIterator, jNextMid);
jclass jIPAddressClass;
ReturnErrorOnFailure(chip::JniReferences::GetInstance().GetClassRef(env, "java/net/InetAddress", jIPAddressClass));
ReturnErrorOnFailure(chip::JniReferences::GetInstance().GetLocalClassRef(env, "java/net/InetAddress", jIPAddressClass));
jmethodID jGetHostAddressMid = env->GetMethodID(jIPAddressClass, "getHostAddress", "()Ljava/lang/String;");
jstring jIPAddressStr = static_cast<jstring>(env->CallObjectMethod(jIPAddress, jGetHostAddressMid));

Expand Down
16 changes: 11 additions & 5 deletions examples/tv-casting-app/android/App/app/src/main/jni/cpp/MatterCallbackHandler-JNI.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,12 @@ jobject CurrentStateSuccessHandlerJNI::ConvertToJObject(
{
ChipLogProgress(AppServer, "CurrentStateSuccessHandlerJNI::ConvertToJObject called");
JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
VerifyOrReturnValue(env != nullptr, nullptr);
JniLocalReferenceManager manager(env);

jclass enumClass = nullptr;
CHIP_ERROR err =
JniReferences::GetInstance().GetClassRef(env, "com/chip/casting/MediaPlaybackTypes$PlaybackStateEnum", enumClass);
JniReferences::GetInstance().GetLocalClassRef(env, "com/chip/casting/MediaPlaybackTypes$PlaybackStateEnum", enumClass);
if (err != CHIP_NO_ERROR)
{
ChipLogError(AppServer, "ConvertToJObject: Class for Response Type not found!");
Expand Down Expand Up @@ -251,6 +253,8 @@ jobject SampledPositionSuccessHandlerJNI::ConvertToJObject(
{
ChipLogProgress(AppServer, "SampledPositionSuccessHandlerJNI::ConvertToJObject called");
JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
VerifyOrReturnValue(env != nullptr, nullptr);
JniLocalReferenceManager manager(env);

jobject jSampledPosition = nullptr;
if (!responseData.IsNull())
Expand All @@ -259,8 +263,8 @@ jobject SampledPositionSuccessHandlerJNI::ConvertToJObject(
responseData.Value();

jclass responseTypeClass = nullptr;
CHIP_ERROR err = JniReferences::GetInstance().GetClassRef(env, "com/chip/casting/MediaPlaybackTypes$PlaybackPositionStruct",
responseTypeClass);
CHIP_ERROR err = JniReferences::GetInstance().GetLocalClassRef(
env, "com/chip/casting/MediaPlaybackTypes$PlaybackPositionStruct", responseTypeClass);
if (err != CHIP_NO_ERROR)
{
ChipLogError(AppServer, "ConvertToJObject: Class for Response Type not found!");
Expand Down Expand Up @@ -318,6 +322,8 @@ jobject TargetListSuccessHandlerJNI::ConvertToJObject(
ChipLogProgress(AppServer, "TargetListSuccessHandlerJNI::ConvertToJObject called");

JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
VerifyOrReturnValue(env != nullptr, nullptr);
JniLocalReferenceManager manager(env);

jobject jArrayList;
chip::JniReferences::GetInstance().CreateArrayList(jArrayList);
Expand All @@ -327,8 +333,8 @@ jobject TargetListSuccessHandlerJNI::ConvertToJObject(
const chip::app::Clusters::TargetNavigator::Structs::TargetInfoStruct::DecodableType & targetInfo = iter.GetValue();

jclass responseTypeClass = nullptr;
CHIP_ERROR err =
JniReferences::GetInstance().GetClassRef(env, "com/chip/casting/TargetNavigatorTypes$TargetInfo", responseTypeClass);
CHIP_ERROR err = JniReferences::GetInstance().GetLocalClassRef(env, "com/chip/casting/TargetNavigatorTypes$TargetInfo",
responseTypeClass);
if (err != CHIP_NO_ERROR)
{
ChipLogError(AppServer, "ConvertToJObject: Class for Response Type not found!");
Expand Down
7 changes: 4 additions & 3 deletions examples/tv-casting-app/android/App/app/src/main/jni/cpp/TvCastingApp-JNI.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ JNI_METHOD(jboolean, openBasicCommissioningWindow)

CommissioningCallbacks commissioningCallbacks;
jclass jCommissioningCallbacksClass;
chip::JniReferences::GetInstance().GetClassRef(env, "com/chip/casting/CommissioningCallbacks", jCommissioningCallbacksClass);
chip::JniReferences::GetInstance().GetLocalClassRef(env, "com/chip/casting/CommissioningCallbacks",
jCommissioningCallbacksClass);

jfieldID jCommissioningCompleteField =
env->GetFieldID(jCommissioningCallbacksClass, "commissioningComplete", "Ljava/lang/Object;");
Expand Down Expand Up @@ -458,8 +459,8 @@ CHIP_ERROR CreateContentSearch(JNIEnv * env, jobject jSearch,
ListFreer & listFreer)
{
jclass jContentSearchClass;
ReturnErrorOnFailure(
JniReferences::GetInstance().GetClassRef(env, "com/chip/casting/ContentLauncherTypes$ContentSearch", jContentSearchClass));
ReturnErrorOnFailure(JniReferences::GetInstance().GetLocalClassRef(env, "com/chip/casting/ContentLauncherTypes$ContentSearch",
jContentSearchClass));

jfieldID jParameterListField = env->GetFieldID(jContentSearchClass, "parameterList", "Ljava/util/ArrayList;");
jobject jParameterList = env->GetObjectField(jSearch, jParameterListField);
Expand Down
2 changes: 1 addition & 1 deletion examples/tv-casting-app/android/App/app/src/main/jni/cpp/core/CastingApp-JNI.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ jobject extractJAppParameter(jobject jAppParameters, const char * methodName, co

jclass jAppParametersClass;
CHIP_ERROR err =
chip::JniReferences::GetInstance().GetClassRef(env, "com/matter/casting/support/AppParameters", jAppParametersClass);
chip::JniReferences::GetInstance().GetLocalClassRef(env, "com/matter/casting/support/AppParameters", jAppParametersClass);
VerifyOrReturnValue(err == CHIP_NO_ERROR, nullptr);

// get the RotatingDeviceIdUniqueIdProvider
Expand Down
2 changes: 1 addition & 1 deletion examples/tv-casting-app/android/App/app/src/main/jni/cpp/support/ErrorConverter-JNI.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobject createJMatterError(CHIP_ERROR inErr)
JNIEnv * env = chip::JniReferences::GetInstance().GetEnvForCurrentThread();
jclass jMatterErrorClass;
CHIP_ERROR err =
chip::JniReferences::GetInstance().GetClassRef(env, "com/matter/casting/support/MatterError", jMatterErrorClass);
chip::JniReferences::GetInstance().GetLocalClassRef(env, "com/matter/casting/support/MatterError", jMatterErrorClass);
VerifyOrReturnValue(err == CHIP_NO_ERROR, nullptr);

jmethodID jMatterErrorConstructor = env->GetMethodID(jMatterErrorClass, "<init>", "(JLjava/lang/String;)V");
Expand Down
65 changes: 3 additions & 62 deletions src/controller/java/AndroidClusterExceptions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,76 +30,17 @@ CHIP_ERROR AndroidClusterExceptions::CreateChipClusterException(JNIEnv * env, ui
jmethodID exceptionConstructor;
jclass clusterExceptionCls;

err = chip::JniReferences::GetInstance().GetClassRef(env, "chip/devicecontroller/ChipClusterException", clusterExceptionCls);
err =
chip::JniReferences::GetInstance().GetLocalClassRef(env, "chip/devicecontroller/ChipClusterException", clusterExceptionCls);
VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_JNI_ERROR_TYPE_NOT_FOUND);
chip::JniClass clusterExceptionJniCls(clusterExceptionCls);

exceptionConstructor = env->GetMethodID(clusterExceptionCls, "<init>", "(J)V");
VerifyOrReturnError(exceptionConstructor != nullptr, CHIP_JNI_ERROR_TYPE_NOT_FOUND);

outEx = (jthrowable) env->NewObject(clusterExceptionCls, exceptionConstructor, static_cast<jlong>(errorCode));
outEx = static_cast<jthrowable>(env->NewObject(clusterExceptionCls, exceptionConstructor, static_cast<jlong>(errorCode)));
VerifyOrReturnError(outEx != nullptr, CHIP_JNI_ERROR_TYPE_NOT_FOUND);

return err;
}

CHIP_ERROR AndroidClusterExceptions::CreateIllegalStateException(JNIEnv * env, const char message[], ChipError errorCode,
jthrowable & outEx)
{
return CreateIllegalStateException(env, message, errorCode.AsInteger(), outEx);
}

CHIP_ERROR AndroidClusterExceptions::CreateIllegalStateException(JNIEnv * env, const char message[],
ChipError::StorageType errorCode, jthrowable & outEx)
{
CHIP_ERROR err = CHIP_NO_ERROR;
jmethodID exceptionConstructor;
jclass exceptionClass;
jstring errStr;

err = JniReferences::GetInstance().GetClassRef(env, "java/lang/IllegalStateException", exceptionClass);
VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_JNI_ERROR_TYPE_NOT_FOUND);
JniClass exceptionJniClass(exceptionClass);

exceptionConstructor = env->GetMethodID(exceptionClass, "<init>", "(Ljava/lang/String;)V");
VerifyOrReturnError(exceptionConstructor != nullptr, CHIP_JNI_ERROR_TYPE_NOT_FOUND);

char buf[CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE];
snprintf(buf, sizeof(buf), "%s: %d", message, errorCode);
errStr = env->NewStringUTF(buf);

outEx = static_cast<jthrowable>(env->NewObject(exceptionClass, exceptionConstructor, errStr));
VerifyOrReturnError(outEx != nullptr, CHIP_JNI_ERROR_TYPE_NOT_FOUND);

return err;
}

void AndroidClusterExceptions::ReturnIllegalStateException(JNIEnv * env, jobject callback, const char message[], ChipError error)
{
ReturnIllegalStateException(env, callback, message, error.AsInteger());
}

void AndroidClusterExceptions::ReturnIllegalStateException(JNIEnv * env, jobject callback, const char message[],
ChipError::StorageType errorCode)
{
VerifyOrReturn(callback != nullptr, ChipLogDetail(Zcl, "Callback is null in ReturnIllegalStateException(), exiting early"));
CHIP_ERROR err = CHIP_NO_ERROR;
jmethodID method;
err = JniReferences::GetInstance().FindMethod(env, callback, "onError", "(Ljava/lang/Exception;)V", &method);
if (err != CHIP_NO_ERROR)
{
ChipLogError(Zcl, "Error throwing IllegalStateException %d", err.AsInteger());
return;
}

jthrowable exception;
err = CreateIllegalStateException(env, message, errorCode, exception);
if (err != CHIP_NO_ERROR)
{
ChipLogError(Zcl, "Error throwing IllegalStateException %d", err.AsInteger());
return;
}
env->CallVoidMethod(callback, method, exception);
}

} // namespace chip
15 changes: 0 additions & 15 deletions src/controller/java/AndroidClusterExceptions.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,6 @@ class AndroidClusterExceptions
*/
CHIP_ERROR CreateChipClusterException(JNIEnv * env, uint32_t errorCode, jthrowable & outEx);

/**
* Creates a Java IllegalStateException in outEx.
*/
CHIP_ERROR CreateIllegalStateException(JNIEnv * env, const char message[], ChipError errorCode, jthrowable & outEx);

CHIP_ERROR CreateIllegalStateException(JNIEnv * env, const char message[], ChipError::StorageType errorCode,
jthrowable & outEx);

/**
* Creates an IllegalStateException and passes it to the Java onError() function of the provided callback object.
*/
void ReturnIllegalStateException(JNIEnv * env, jobject callback, const char message[], ChipError errorCode);

void ReturnIllegalStateException(JNIEnv * env, jobject callback, const char message[], ChipError::StorageType errorCode);

private:
AndroidClusterExceptions() {}
};
Expand Down
12 changes: 7 additions & 5 deletions src/controller/java/AndroidDeviceControllerWrapper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -751,8 +751,10 @@ void AndroidDeviceControllerWrapper::OnScanNetworksSuccess(
{
chip::DeviceLayer::StackUnlock unlock;
CHIP_ERROR err = CHIP_NO_ERROR;
JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
jmethodID javaMethod;
JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
VerifyOrReturn(env != nullptr, ChipLogError(Controller, "Could not get JNIEnv for current thread"));
JniLocalReferenceManager manager(env);

VerifyOrReturn(env != nullptr, ChipLogError(Zcl, "Error invoking Java callback: no JNIEnv"));

Expand Down Expand Up @@ -822,8 +824,8 @@ void AndroidDeviceControllerWrapper::OnScanNetworksSuccess(
chip::JniReferences::GetInstance().CreateBoxedObject<jint>("java/lang/Integer", "(I)V", jniRssi, newElement_rssi);

jclass wiFiInterfaceScanResultStructClass;
err = chip::JniReferences::GetInstance().GetClassRef(env, "chip/devicecontroller/WiFiScanResult",
wiFiInterfaceScanResultStructClass);
err = chip::JniReferences::GetInstance().GetLocalClassRef(env, "chip/devicecontroller/WiFiScanResult",
wiFiInterfaceScanResultStructClass);
if (err != CHIP_NO_ERROR)
{
ChipLogError(Zcl, "Could not find class WiFiScanResult");
Expand Down Expand Up @@ -887,8 +889,8 @@ void AndroidDeviceControllerWrapper::OnScanNetworksSuccess(
chip::JniReferences::GetInstance().CreateBoxedObject<jint>("java/lang/Integer", "(I)V", jniLqi, newElement_lqi);

jclass threadInterfaceScanResultStructClass;
err = chip::JniReferences::GetInstance().GetClassRef(env, "chip/devicecontroller/ThreadScanResult",
threadInterfaceScanResultStructClass);
err = chip::JniReferences::GetInstance().GetLocalClassRef(env, "chip/devicecontroller/ThreadScanResult",
threadInterfaceScanResultStructClass);
if (err != CHIP_NO_ERROR)
{
ChipLogError(Zcl, "Could not find class ThreadScanResult");
Expand Down
Loading

0 comments on commit 1023378

Please sign in to comment.