From 03079387da09c1588d84015b6955e484dccfe69b Mon Sep 17 00:00:00 2001 From: Amy Zhang Date: Mon, 22 Feb 2021 18:22:12 -0800 Subject: [PATCH] Check if the weak reference of java object is alive before using it This CL get the global strong reference from the weak reference before calling methods on a weak java object reference in Tuner JNI. The method call would use the strong ref instead of the weak ref after making sure it's non-null because weak ref is not garanteed non-null even after the null check see https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#weak_global_references for more details Test: atest android.media.tv.tuner.cts Bug: 176190716 Change-Id: I81215c18cda413c5ddee0e46480ce3783dfe4be9 --- media/jni/android_media_tv_Tuner.cpp | 177 +++++++++++++++++++-------- media/jni/android_media_tv_Tuner.h | 3 +- 2 files changed, 130 insertions(+), 50 deletions(-) diff --git a/media/jni/android_media_tv_Tuner.cpp b/media/jni/android_media_tv_Tuner.cpp index eee9f1e08131d..1459a6b46b2ba 100644 --- a/media/jni/android_media_tv_Tuner.cpp +++ b/media/jni/android_media_tv_Tuner.cpp @@ -217,23 +217,36 @@ namespace android { void LnbClientCallbackImpl::onEvent(const LnbEventType lnbEventType) { ALOGD("LnbClientCallbackImpl::onEvent, type=%d", lnbEventType); JNIEnv *env = AndroidRuntime::getJNIEnv(); - env->CallVoidMethod( - mLnbObj, - gFields.onLnbEventID, - (jint)lnbEventType); + jobject lnb(env->NewLocalRef(mLnbObj)); + if (!env->IsSameObject(lnb, nullptr)) { + env->CallVoidMethod( + lnb, + gFields.onLnbEventID, + (jint)lnbEventType); + } else { + ALOGE("LnbClientCallbackImpl::onEvent:" + "Lnb object has been freed. Ignoring callback."); + env->DeleteWeakGlobalRef(mLnbObj); + } } void LnbClientCallbackImpl::onDiseqcMessage(const hidl_vec& diseqcMessage) { ALOGD("LnbClientCallbackImpl::onDiseqcMessage"); JNIEnv *env = AndroidRuntime::getJNIEnv(); - jbyteArray array = env->NewByteArray(diseqcMessage.size()); - env->SetByteArrayRegion( - array, 0, diseqcMessage.size(), reinterpret_cast(diseqcMessage[0])); - - env->CallVoidMethod( - mLnbObj, - gFields.onLnbDiseqcMessageID, - array); + jobject lnb(env->NewLocalRef(mLnbObj)); + if (!env->IsSameObject(lnb, nullptr)) { + jbyteArray array = env->NewByteArray(diseqcMessage.size()); + env->SetByteArrayRegion( + array, 0, diseqcMessage.size(), reinterpret_cast(diseqcMessage[0])); + env->CallVoidMethod( + lnb, + gFields.onLnbDiseqcMessageID, + array); + } else { + ALOGE("LnbClientCallbackImpl::onDiseqcMessage:" + "Lnb object has been freed. Ignoring callback."); + env->DeleteWeakGlobalRef(mLnbObj); + } } void LnbClientCallbackImpl::setLnb(jweak lnbObj) { @@ -254,19 +267,33 @@ LnbClientCallbackImpl::~LnbClientCallbackImpl() { void DvrClientCallbackImpl::onRecordStatus(RecordStatus status) { ALOGD("DvrClientCallbackImpl::onRecordStatus"); JNIEnv *env = AndroidRuntime::getJNIEnv(); - env->CallVoidMethod( - mDvrObj, - gFields.onDvrRecordStatusID, - (jint) status); + jobject dvr(env->NewLocalRef(mDvrObj)); + if (!env->IsSameObject(dvr, nullptr)) { + env->CallVoidMethod( + dvr, + gFields.onDvrRecordStatusID, + (jint) status); + } else { + ALOGE("DvrClientCallbackImpl::onRecordStatus:" + "Dvr object has been freed. Ignoring callback."); + env->DeleteWeakGlobalRef(mDvrObj); + } } void DvrClientCallbackImpl::onPlaybackStatus(PlaybackStatus status) { ALOGD("DvrClientCallbackImpl::onPlaybackStatus"); JNIEnv *env = AndroidRuntime::getJNIEnv(); - env->CallVoidMethod( - mDvrObj, - gFields.onDvrPlaybackStatusID, - (jint) status); + jobject dvr(env->NewLocalRef(mDvrObj)); + if (!env->IsSameObject(dvr, nullptr)) { + env->CallVoidMethod( + dvr, + gFields.onDvrPlaybackStatusID, + (jint) status); + } else { + ALOGE("DvrClientCallbackImpl::onPlaybackStatus:" + "Dvr object has been freed. Ignoring callback."); + env->DeleteWeakGlobalRef(mDvrObj); + } } void DvrClientCallbackImpl::setDvr(jweak dvrObj) { @@ -810,10 +837,17 @@ void FilterClientCallbackImpl::onFilterEvent_1_1(const DemuxFilterEvent& filterE } } } - env->CallVoidMethod( - mFilterObj, - gFields.onFilterEventID, - array); + jobject filter(env->NewLocalRef(mFilterObj)); + if (!env->IsSameObject(filter, nullptr)) { + env->CallVoidMethod( + filter, + gFields.onFilterEventID, + array); + } else { + ALOGE("FilterClientCallbackImpl::onFilterEvent_1_1:" + "Filter object has been freed. Ignoring callback."); + env->DeleteWeakGlobalRef(mFilterObj); + } } void FilterClientCallbackImpl::onFilterEvent(const DemuxFilterEvent& filterEvent) { @@ -828,10 +862,17 @@ void FilterClientCallbackImpl::onFilterEvent(const DemuxFilterEvent& filterEvent void FilterClientCallbackImpl::onFilterStatus(const DemuxFilterStatus status) { ALOGD("FilterClientCallbackImpl::onFilterStatus"); JNIEnv *env = AndroidRuntime::getJNIEnv(); - env->CallVoidMethod( - mFilterObj, - gFields.onFilterStatusID, - (jint)status); + jobject filter(env->NewLocalRef(mFilterObj)); + if (!env->IsSameObject(filter, nullptr)) { + env->CallVoidMethod( + filter, + gFields.onFilterStatusID, + (jint)status); + } else { + ALOGE("FilterClientCallbackImpl::onFilterStatus:" + "Filter object has been freed. Ignoring callback."); + env->DeleteWeakGlobalRef(mFilterObj); + } } void FilterClientCallbackImpl::setFilter(jweak filterObj, sp filterClient) { @@ -841,6 +882,15 @@ void FilterClientCallbackImpl::setFilter(jweak filterObj, sp filte mFilterClient = filterClient; } +FilterClientCallbackImpl::~FilterClientCallbackImpl() { + JNIEnv *env = AndroidRuntime::getJNIEnv(); + if (mFilterObj != NULL) { + env->DeleteWeakGlobalRef(mFilterObj); + mFilterObj = NULL; + } + mFilterClient = NULL; +} + /////////////// FrontendClientCallbackImpl /////////////////////// FrontendClientCallbackImpl::FrontendClientCallbackImpl(jweak tunerObj) : mObject(tunerObj) {} @@ -848,10 +898,17 @@ FrontendClientCallbackImpl::FrontendClientCallbackImpl(jweak tunerObj) : mObject void FrontendClientCallbackImpl::onEvent(FrontendEventType frontendEventType) { ALOGD("FrontendClientCallbackImpl::onEvent, type=%d", frontendEventType); JNIEnv *env = AndroidRuntime::getJNIEnv(); - env->CallVoidMethod( - mObject, - gFields.onFrontendEventID, - (jint)frontendEventType); + jobject frontend(env->NewLocalRef(mObject)); + if (!env->IsSameObject(frontend, nullptr)) { + env->CallVoidMethod( + frontend, + gFields.onFrontendEventID, + (jint)frontendEventType); + } else { + ALOGE("FrontendClientCallbackImpl::onEvent:" + "Frontend object has been freed. Ignoring callback."); + env->DeleteWeakGlobalRef(mObject); + } } void FrontendClientCallbackImpl::onScanMessage( @@ -859,11 +916,18 @@ void FrontendClientCallbackImpl::onScanMessage( ALOGD("FrontendClientCallbackImpl::onScanMessage, type=%d", type); JNIEnv *env = AndroidRuntime::getJNIEnv(); jclass clazz = env->FindClass("android/media/tv/tuner/Tuner"); + jobject frontend(env->NewLocalRef(mObject)); + if (env->IsSameObject(frontend, nullptr)) { + ALOGE("FrontendClientCallbackImpl::onScanMessage:" + "Frontend object has been freed. Ignoring callback."); + env->DeleteWeakGlobalRef(mObject); + return; + } switch(type) { case FrontendScanMessageType::LOCKED: { if (message.isLocked()) { env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onLocked", "()V")); } break; @@ -871,14 +935,14 @@ void FrontendClientCallbackImpl::onScanMessage( case FrontendScanMessageType::END: { if (message.isEnd()) { env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onScanStopped", "()V")); } break; } case FrontendScanMessageType::PROGRESS_PERCENT: { env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onProgress", "(I)V"), (jint) message.progressPercent()); break; @@ -889,7 +953,7 @@ void FrontendClientCallbackImpl::onScanMessage( env->SetIntArrayRegion(freqs, 0, v.size(), reinterpret_cast(&v[0])); env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onFrequenciesReport", "([I)V"), freqs); break; @@ -900,21 +964,21 @@ void FrontendClientCallbackImpl::onScanMessage( env->SetIntArrayRegion(symbolRates, 0, v.size(), reinterpret_cast(&v[0])); env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onSymbolRates", "([I)V"), symbolRates); break; } case FrontendScanMessageType::HIERARCHY: { env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onHierarchy", "(I)V"), (jint) message.hierarchy()); break; } case FrontendScanMessageType::ANALOG_TYPE: { env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onSignalType", "(I)V"), (jint) message.analogType()); break; @@ -926,7 +990,7 @@ void FrontendClientCallbackImpl::onScanMessage( env->SetIntArrayRegion(plpIds, 0, jintV.size(), &jintV[0]); env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onPlpIds", "([I)V"), plpIds); break; @@ -938,7 +1002,7 @@ void FrontendClientCallbackImpl::onScanMessage( env->SetIntArrayRegion(groupIds, 0, jintV.size(), &jintV[0]); env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onGroupIds", "([I)V"), groupIds); break; @@ -950,7 +1014,7 @@ void FrontendClientCallbackImpl::onScanMessage( env->SetIntArrayRegion(streamIds, 0, jintV.size(), &jintV[0]); env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onInputStreamIds", "([I)V"), streamIds); break; @@ -961,21 +1025,21 @@ void FrontendClientCallbackImpl::onScanMessage( if (std.getDiscriminator() == FrontendScanMessage::Standard::hidl_discriminator::sStd) { standard = (jint) std.sStd(); env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onDvbsStandard", "(I)V"), standard); } else if (std.getDiscriminator() == FrontendScanMessage::Standard::hidl_discriminator::tStd) { standard = (jint) std.tStd(); env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onDvbtStandard", "(I)V"), standard); } else if (std.getDiscriminator() == FrontendScanMessage::Standard::hidl_discriminator::sifStd) { standard = (jint) std.sifStd(); env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onAnalogSifStandard", "(I)V"), standard); } @@ -996,7 +1060,7 @@ void FrontendClientCallbackImpl::onScanMessage( env->SetObjectArrayElement(array, i, obj); } env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onAtsc3PlpInfos", "([Landroid/media/tv/tuner/frontend/Atsc3PlpInfo;)V"), array); @@ -1010,6 +1074,13 @@ void FrontendClientCallbackImpl::onScanMessageExt1_1(FrontendScanMessageTypeExt1 ALOGD("FrontendClientCallbackImpl::onScanMessageExt1_1, type=%d", type); JNIEnv *env = AndroidRuntime::getJNIEnv(); jclass clazz = env->FindClass("android/media/tv/tuner/Tuner"); + jobject frontend(env->NewLocalRef(mObject)); + if (env->IsSameObject(frontend, nullptr)) { + ALOGE("FrontendClientCallbackImpl::onScanMessageExt1_1:" + "Frontend object has been freed. Ignoring callback."); + env->DeleteWeakGlobalRef(mObject); + return; + } switch(type) { case FrontendScanMessageTypeExt1_1::MODULATION: { jint modulation = -1; @@ -1056,7 +1127,7 @@ void FrontendClientCallbackImpl::onScanMessageExt1_1(FrontendScanMessageTypeExt1 } if (modulation > 0) { env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onModulationReported", "(I)V"), modulation); } @@ -1065,7 +1136,7 @@ void FrontendClientCallbackImpl::onScanMessageExt1_1(FrontendScanMessageTypeExt1 case FrontendScanMessageTypeExt1_1::HIGH_PRIORITY: { bool isHighPriority = message.isHighPriority(); env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onPriorityReported", "(B)V"), isHighPriority); break; @@ -1073,7 +1144,7 @@ void FrontendClientCallbackImpl::onScanMessageExt1_1(FrontendScanMessageTypeExt1 case FrontendScanMessageTypeExt1_1::DVBC_ANNEX: { jint dvbcAnnex = (jint) message.annex(); env->CallVoidMethod( - mObject, + frontend, env->GetMethodID(clazz, "onDvbcAnnexReported", "(I)V"), dvbcAnnex); break; @@ -1083,6 +1154,14 @@ void FrontendClientCallbackImpl::onScanMessageExt1_1(FrontendScanMessageTypeExt1 } } +FrontendClientCallbackImpl::~FrontendClientCallbackImpl() { + JNIEnv *env = AndroidRuntime::getJNIEnv(); + if (mObject != NULL) { + env->DeleteWeakGlobalRef(mObject); + mObject = NULL; + } +} + /////////////// Tuner /////////////////////// sp JTuner::mTunerClient; diff --git a/media/jni/android_media_tv_Tuner.h b/media/jni/android_media_tv_Tuner.h index 0e30b18eb2d45..fafef42215415 100644 --- a/media/jni/android_media_tv_Tuner.h +++ b/media/jni/android_media_tv_Tuner.h @@ -118,6 +118,7 @@ struct MediaEvent : public RefBase { }; struct FilterClientCallbackImpl : public FilterClientCallback { + ~FilterClientCallbackImpl(); virtual void onFilterEvent_1_1(const DemuxFilterEvent& filterEvent, const DemuxFilterEventExt& filterEventExt); virtual void onFilterEvent(const DemuxFilterEvent& filterEvent); @@ -155,7 +156,7 @@ private: struct FrontendClientCallbackImpl : public FrontendClientCallback { FrontendClientCallbackImpl(jweak tunerObj); - + ~FrontendClientCallbackImpl(); virtual void onEvent(FrontendEventType frontendEventType); virtual void onScanMessage( FrontendScanMessageType type, const FrontendScanMessage& message);