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 authored and pull[bot] committed Apr 10, 2024
1 parent d421999 commit 1397298
Show file tree
Hide file tree
Showing 21 changed files with 330 additions and 407 deletions.
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
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
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
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
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
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
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
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 1397298

Please sign in to comment.