Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

AudioEffect: fix racy access to callback data

Browse Source 
Since the EffectCallback in android::AudioEffect can
keep an instance of the latter alive for a longer time
than Java AudioEffect is alive, the callback need to
ensure that it's not trying to use deallocated objects
and deleted JVM references.

Bug: 178363662
Test: manual using EffectsTest app
Test: atest CtsMediaTestCases:AudioEffectTest
Test: atest CtsMediaTestCases:BassBoostTest
Test: atest CtsMediaTestCases:DynamicsProcessingTest
Test: atest CtsMediaTestCases:EnvReverbTest
Test: atest CtsMediaTestCases:EqualizerTest
Test: atest CtsMediaTestCases:LoudnessEnhancerTest
Test: atest CtsMediaTestCases:PresetReverbTest
Test: atest CtsMediaTestCases:VisualizerTest
Change-Id: I8c36f5af4a1a2b14e81b1344570d2f8746ffa91e
This commit is contained in:
Mikhail Naganov
2021-09-14 10:58:48 -07:00
parent 7d6d9bd6c3
commit 9690a02f80
2 changed files with 110 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

52
media/jni/audioeffect/android_media_AudioEffect.cpp
View File

@@ -14,8 +14,8 @@
* limitations under the License.
*/
#include <stdio.h>
#include <unordered_set>
//#define LOG_NDEBUG 0
#define LOG_TAG "AudioEffects-JNI"
@@ -58,21 +58,14 @@ static fields_t fields;
struct effect_callback_cookie {
jclass audioEffect_class; // AudioEffect class
jobject audioEffect_ref; // AudioEffect object instance
};
// ----------------------------------------------------------------------------
class AudioEffectJniStorage {
public:
effect_callback_cookie mCallbackData;
AudioEffectJniStorage() {
}
~AudioEffectJniStorage() {
}
bool busy;
Condition cond;
};
// ----------------------------------------------------------------------------
struct AudioEffectJniStorage {
effect_callback_cookie mCallbackData{};
};
jint AudioEffectJni::translateNativeErrorToJava(int code) {
switch(code) {
@@ -101,6 +94,7 @@ jint AudioEffectJni::translateNativeErrorToJava(int code) {
}
static Mutex sLock;
static std::unordered_set<effect_callback_cookie*> sAudioEffectCallBackCookies;
// ----------------------------------------------------------------------------
static void effectCallback(int event, void* user, void *info) {
@@ -121,7 +115,13 @@ static void effectCallback(int event, void* user, void *info) {
ALOGW("effectCallback error user %p, env %p", user, env);
return;
}
{
Mutex::Autolock l(sLock);
if (sAudioEffectCallBackCookies.count(callbackInfo) == 0) {
return;
}
callbackInfo->busy = true;
}
ALOGV("effectCallback: callbackInfo %p, audioEffect_ref %p audioEffect_class %p",
callbackInfo,
callbackInfo->audioEffect_ref,
@@ -188,6 +188,11 @@ effectCallback_Exit:
env->ExceptionDescribe();
env->ExceptionClear();
}
{
Mutex::Autolock l(sLock);
callbackInfo->busy = false;
callbackInfo->cond.broadcast();
}
}
// ----------------------------------------------------------------------------
@@ -396,6 +401,10 @@ android_media_AudioEffect_native_setup(JNIEnv *env, jobject thiz, jobject weak_t
setAudioEffect(env, thiz, lpAudioEffect);
}
{
Mutex::Autolock l(sLock);
sAudioEffectCallBackCookies.insert(&lpJniStorage->mCallbackData);
}
env->SetLongField(thiz, fields.fidJniData, (jlong)lpJniStorage);
return (jint) AUDIOEFFECT_SUCCESS;
@@ -427,6 +436,7 @@ setup_failure:
// ----------------------------------------------------------------------------
#define CALLBACK_COND_WAIT_TIMEOUT_MS 1000
static void android_media_AudioEffect_native_release(JNIEnv *env, jobject thiz) {
sp<AudioEffect> lpAudioEffect = setAudioEffect(env, thiz, 0);
if (lpAudioEffect == 0) {
@@ -442,7 +452,17 @@ static void android_media_AudioEffect_native_release(JNIEnv *env, jobject thiz)
env->SetLongField(thiz, fields.fidJniData, 0);
if (lpJniStorage) {
ALOGV("deleting pJniStorage: %p\n", lpJniStorage);
Mutex::Autolock l(sLock);
effect_callback_cookie *lpCookie = &lpJniStorage->mCallbackData;
ALOGV("deleting lpJniStorage: %p\n", lpJniStorage);
sAudioEffectCallBackCookies.erase(lpCookie);
while (lpCookie->busy) {
if (lpCookie->cond.waitRelative(sLock,
milliseconds(CALLBACK_COND_WAIT_TIMEOUT_MS)) !=
NO_ERROR) {
break;
}
}
env->DeleteGlobalRef(lpJniStorage->mCallbackData.audioEffect_class);
env->DeleteGlobalRef(lpJniStorage->mCallbackData.audioEffect_ref);
delete lpJniStorage;

92
media/jni/audioeffect/android_media_Visualizer.cpp
View File

@@ -15,6 +15,7 @@
*/
#include <stdio.h>
#include <unordered_set>
//#define LOG_NDEBUG 0
#define LOG_TAG "visualizers-JNI"
@@ -63,6 +64,12 @@ struct visualizer_callback_cookie {
jclass visualizer_class; // Visualizer class
jobject visualizer_ref; // Visualizer object instance
// 'busy_count' and 'cond' together with 'sLock' are used to serialize
// concurrent access to the callback cookie from 'setup'/'release'
// and the callback.
int busy_count;
Condition cond;
// Lazily allocated arrays used to hold callback data provided to java
// applications. These arrays are allocated during the first callback and
// reallocated when the size of the callback data changes. Allocating on
@@ -70,14 +77,12 @@ struct visualizer_callback_cookie {
// reference to the provided data (they need to make a copy if they want to
// hold onto outside of the callback scope), but it avoids GC thrash caused
// by constantly allocating and releasing arrays to hold callback data.
// 'callback_data_lock' must never be held at the same time with 'sLock'.
Mutex callback_data_lock;
jbyteArray waveform_data;
jbyteArray fft_data;
visualizer_callback_cookie() {
waveform_data = NULL;
fft_data = NULL;
}
// Assumes use of default initialization by the client.
~visualizer_callback_cookie() {
cleanupBuffers();
@@ -102,15 +107,8 @@ struct visualizer_callback_cookie {
};
// ----------------------------------------------------------------------------
class VisualizerJniStorage {
public:
visualizer_callback_cookie mCallbackData;
VisualizerJniStorage() {
}
~VisualizerJniStorage() {
}
struct VisualizerJniStorage {
visualizer_callback_cookie mCallbackData{};
};
@@ -136,6 +134,7 @@ static jint translateError(int code) {
}
static Mutex sLock;
static std::unordered_set<visualizer_callback_cookie*> sVisualizerCallBackCookies;
// ----------------------------------------------------------------------------
static void ensureArraySize(JNIEnv *env, jbyteArray *array, uint32_t size) {
@@ -173,11 +172,19 @@ static void captureCallback(void* user,
return;
}
{
Mutex::Autolock l(sLock);
if (sVisualizerCallBackCookies.count(callbackInfo) == 0) {
return;
}
callbackInfo->busy_count++;
}
ALOGV("captureCallback: callbackInfo %p, visualizer_ref %p visualizer_class %p",
callbackInfo,
callbackInfo->visualizer_ref,
callbackInfo->visualizer_class);
{
AutoMutex lock(&callbackInfo->callback_data_lock);
if (waveformSize != 0 && waveform != NULL) {
@@ -219,11 +226,17 @@ static void captureCallback(void* user,
jArray);
}
}
} // callback_data_lock scope
if (env->ExceptionCheck()) {
env->ExceptionDescribe();
env->ExceptionClear();
}
{
Mutex::Autolock l(sLock);
callbackInfo->busy_count--;
callbackInfo->cond.broadcast();
}
}
// ----------------------------------------------------------------------------
@@ -332,16 +345,41 @@ static void android_media_visualizer_effect_callback(int32_t event,
void *info) {
if ((event == AudioEffect::EVENT_ERROR) &&
(*((status_t*)info) == DEAD_OBJECT)) {
VisualizerJniStorage* lpJniStorage = (VisualizerJniStorage*)user;
visualizer_callback_cookie* callbackInfo = &lpJniStorage->mCallbackData;
visualizer_callback_cookie* callbackInfo =
(visualizer_callback_cookie *)user;
JNIEnv *env = AndroidRuntime::getJNIEnv();
if (!user || !env) {
ALOGW("effectCallback error user %p, env %p", user, env);
return;
}
{
Mutex::Autolock l(sLock);
if (sVisualizerCallBackCookies.count(callbackInfo) == 0) {
return;
}
callbackInfo->busy_count++;
}
ALOGV("effectCallback: callbackInfo %p, visualizer_ref %p visualizer_class %p",
callbackInfo,
callbackInfo->visualizer_ref,
callbackInfo->visualizer_class);
env->CallStaticVoidMethod(
callbackInfo->visualizer_class,
fields.midPostNativeEvent,
callbackInfo->visualizer_ref,
NATIVE_EVENT_SERVER_DIED,
0, NULL);
if (env->ExceptionCheck()) {
env->ExceptionDescribe();
env->ExceptionClear();
}
{
Mutex::Autolock l(sLock);
callbackInfo->busy_count--;
callbackInfo->cond.broadcast();
}
}
}
@@ -389,7 +427,7 @@ android_media_visualizer_native_setup(JNIEnv *env, jobject thiz, jobject weak_th
}
lpVisualizer->set(0,
android_media_visualizer_effect_callback,
lpJniStorage,
&lpJniStorage->mCallbackData,
(audio_session_t) sessionId);
lStatus = translateError(lpVisualizer->initCheck());
@@ -410,6 +448,10 @@ android_media_visualizer_native_setup(JNIEnv *env, jobject thiz, jobject weak_th
setVisualizer(env, thiz, lpVisualizer);
{
Mutex::Autolock l(sLock);
sVisualizerCallBackCookies.insert(&lpJniStorage->mCallbackData);
}
env->SetLongField(thiz, fields.fidJniData, (jlong)lpJniStorage);
return VISUALIZER_SUCCESS;
@@ -432,13 +474,15 @@ setup_failure:
}
// ----------------------------------------------------------------------------
#define CALLBACK_COND_WAIT_TIMEOUT_MS 1000
static void android_media_visualizer_native_release(JNIEnv *env, jobject thiz) {
{ //limit scope so that lpVisualizer is deleted before JNI storage data.
{
sp<Visualizer> lpVisualizer = setVisualizer(env, thiz, 0);
if (lpVisualizer == 0) {
return;
}
lpVisualizer->release();
// Visualizer can still can be held by AudioEffect::EffectClient
}
// delete the JNI data
VisualizerJniStorage* lpJniStorage =
@@ -449,9 +493,22 @@ static void android_media_visualizer_native_release(JNIEnv *env, jobject thiz)
env->SetLongField(thiz, fields.fidJniData, 0);
if (lpJniStorage) {
{
Mutex::Autolock l(sLock);
visualizer_callback_cookie *lpCookie = &lpJniStorage->mCallbackData;
ALOGV("deleting pJniStorage: %p\n", lpJniStorage);
sVisualizerCallBackCookies.erase(lpCookie);
while (lpCookie->busy_count > 0) {
if (lpCookie->cond.waitRelative(sLock,
milliseconds(CALLBACK_COND_WAIT_TIMEOUT_MS)) !=
NO_ERROR) {
break;
}
}
ALOG_ASSERT(lpCookie->busy_count == 0, "Unbalanced busy_count inc/dec");
env->DeleteGlobalRef(lpJniStorage->mCallbackData.visualizer_class);
env->DeleteGlobalRef(lpJniStorage->mCallbackData.visualizer_ref);
} // sLock scope
delete lpJniStorage;
}
}
@@ -707,4 +764,3 @@ int register_android_media_visualizer(JNIEnv *env)
{
return AndroidRuntime::registerNativeMethods(env, kClassPathName, gMethods, NELEM(gMethods));
}