Fix addAudioDeviceCallback() memory leak bug
Store jobjects and JObjectHolders as backup, delete JObjectHolders after
usage
Bug: 128341809
Test: RoutingTest#test_MediaPlayer2_RoutingChangedCallback
Change-Id: I70b2312519ab0efef0ed40ce721ae8c26dc1aed2
diff --git a/media/libmediaplayer2/JAudioTrack.cpp b/media/libmediaplayer2/JAudioTrack.cpp
index 910edff..fab6c64 100644
--- a/media/libmediaplayer2/JAudioTrack.cpp
+++ b/media/libmediaplayer2/JAudioTrack.cpp
@@ -571,10 +571,9 @@
}
void JAudioTrack::registerRoutingDelegates(
- Vector<std::pair<jobject, jobject>>& routingDelegates) {
- for (Vector<std::pair<jobject, jobject>>::iterator it = routingDelegates.begin();
- it != routingDelegates.end(); it++) {
- addAudioDeviceCallback(it->second, getHandler(it->second));
+ Vector<std::pair<sp<JObjectHolder>, sp<JObjectHolder>>>& routingDelegates) {
+ for (auto it = routingDelegates.begin(); it != routingDelegates.end(); it++) {
+ addAudioDeviceCallback(it->second->getJObject(), getHandler(it->second->getJObject()));
}
}
@@ -597,20 +596,22 @@
return env->CallObjectMethod(routingDelegateObj, jGetHandler);
}
-jobject JAudioTrack::findByKey(Vector<std::pair<jobject, jobject>>& mp, const jobject key) {
+jobject JAudioTrack::findByKey(
+ Vector<std::pair<sp<JObjectHolder>, sp<JObjectHolder>>>& mp, const jobject key) {
JNIEnv *env = JavaVMHelper::getJNIEnv();
- for (Vector<std::pair<jobject, jobject>>::iterator it = mp.begin(); it != mp.end(); it++) {
- if (env->IsSameObject(it->first, key)) {
- return it->second;
+ for (auto it = mp.begin(); it != mp.end(); it++) {
+ if (env->IsSameObject(it->first->getJObject(), key)) {
+ return it->second->getJObject();
}
}
return nullptr;
}
-void JAudioTrack::eraseByKey(Vector<std::pair<jobject, jobject>>& mp, const jobject key) {
+void JAudioTrack::eraseByKey(
+ Vector<std::pair<sp<JObjectHolder>, sp<JObjectHolder>>>& mp, const jobject key) {
JNIEnv *env = JavaVMHelper::getJNIEnv();
- for (Vector<std::pair<jobject, jobject>>::iterator it = mp.begin(); it != mp.end(); it++) {
- if (env->IsSameObject(it->first, key)) {
+ for (auto it = mp.begin(); it != mp.end(); it++) {
+ if (env->IsSameObject(it->first->getJObject(), key)) {
mp.erase(it);
return;
}
diff --git a/media/libmediaplayer2/MediaPlayer2AudioOutput.cpp b/media/libmediaplayer2/MediaPlayer2AudioOutput.cpp
index 7c3063d..b4fa0c1 100644
--- a/media/libmediaplayer2/MediaPlayer2AudioOutput.cpp
+++ b/media/libmediaplayer2/MediaPlayer2AudioOutput.cpp
@@ -521,15 +521,18 @@
status_t MediaPlayer2AudioOutput::addAudioDeviceCallback(jobject jRoutingDelegate) {
ALOGV("addAudioDeviceCallback");
Mutex::Autolock lock(mLock);
- jobject listener = (new JObjectHolder(
- JAudioTrack::getListener(jRoutingDelegate)))->getJObject();
+ jobject listener = JAudioTrack::getListener(jRoutingDelegate);
if (JAudioTrack::findByKey(mRoutingDelegates, listener) == nullptr) {
- jobject handler = (new JObjectHolder(
- JAudioTrack::getHandler(jRoutingDelegate)))->getJObject();
- jobject routingDelegate = (new JObjectHolder(jRoutingDelegate))->getJObject();
- mRoutingDelegates.push_back(std::pair<jobject, jobject>(listener, routingDelegate));
+ sp<JObjectHolder> listenerHolder = new JObjectHolder(listener);
+ jobject handler = JAudioTrack::getHandler(jRoutingDelegate);
+ sp<JObjectHolder> routingDelegateHolder = new JObjectHolder(jRoutingDelegate);
+
+ mRoutingDelegates.push_back(std::pair<sp<JObjectHolder>, sp<JObjectHolder>>(
+ listenerHolder, routingDelegateHolder));
+
if (mJAudioTrack != nullptr) {
- return mJAudioTrack->addAudioDeviceCallback(routingDelegate, handler);
+ return mJAudioTrack->addAudioDeviceCallback(
+ routingDelegateHolder->getJObject(), handler);
}
}
return NO_ERROR;
diff --git a/media/libmediaplayer2/include/mediaplayer2/JAudioTrack.h b/media/libmediaplayer2/include/mediaplayer2/JAudioTrack.h
index 7381286..2ed4632 100644
--- a/media/libmediaplayer2/include/mediaplayer2/JAudioTrack.h
+++ b/media/libmediaplayer2/include/mediaplayer2/JAudioTrack.h
@@ -405,7 +405,8 @@
* routingDelegates: backed-up routing delegates
*
*/
- void registerRoutingDelegates(Vector<std::pair<jobject, jobject>>& routingDelegates);
+ void registerRoutingDelegates(
+ Vector<std::pair<sp<JObjectHolder>, sp<JObjectHolder>>>& routingDelegates);
/* get listener from RoutingDelegate object
*/
@@ -422,13 +423,15 @@
* Returns value if key is in the map
* nullptr if key is not in the map
*/
- static jobject findByKey(Vector<std::pair<jobject, jobject>>& mp, const jobject key);
+ static jobject findByKey(
+ Vector<std::pair<sp<JObjectHolder>, sp<JObjectHolder>>>& mp, const jobject key);
/*
* Parameters:
* map and key
*/
- static void eraseByKey(Vector<std::pair<jobject, jobject>>& mp, const jobject key);
+ static void eraseByKey(
+ Vector<std::pair<sp<JObjectHolder>, sp<JObjectHolder>>>& mp, const jobject key);
private:
audio_output_flags_t mFlags;
diff --git a/media/libmediaplayer2/include/mediaplayer2/MediaPlayer2AudioOutput.h b/media/libmediaplayer2/include/mediaplayer2/MediaPlayer2AudioOutput.h
index 1b3f2dc..f38b7cc 100644
--- a/media/libmediaplayer2/include/mediaplayer2/MediaPlayer2AudioOutput.h
+++ b/media/libmediaplayer2/include/mediaplayer2/MediaPlayer2AudioOutput.h
@@ -124,7 +124,9 @@
audio_output_flags_t mFlags;
sp<JObjectHolder> mPreferredDevice;
mutable Mutex mLock;
- Vector<std::pair<jobject, jobject>> mRoutingDelegates; // <listener, routingDelegate>
+
+ // <listener, routingDelegate>
+ Vector<std::pair<sp<JObjectHolder>, sp<JObjectHolder>>> mRoutingDelegates;
// static variables below not protected by mutex
static bool mIsOnEmulator;