Skip to content

Commit 532bc1c

Browse files
author
Eric Laurent
committed
Fix threading issues in AudioRecord JNI
Made native AudioRecord ref based to allow use of strong pointers in JNI and solve concurrency issues between read() and release() in particular. Applied the same fixes to AudioTrack JNI. Issue 6254582. Change-Id: I381a66cb00b6639f87f4fcd19a2e202d1a4e6704
1 parent 0b568ff commit 532bc1c

File tree

2 files changed

+248
-226
lines changed

2 files changed

+248
-226
lines changed

‎core/jni/android_media_AudioRecord.cpp‎

Lines changed: 96 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
#include <android_runtime/AndroidRuntime.h>
2929

3030
#include <utils/Log.h>
31+
#include <utils/SortedVector.h>
32+
#include <utils/threads.h>
3133
#include <media/AudioRecord.h>
3234
#include <media/mediarecorder.h>
3335

@@ -55,9 +57,12 @@ static fields_t javaAudioRecordFields;
5557
struct audiorecord_callback_cookie {
5658
jclass audioRecord_class;
5759
jobject audioRecord_ref;
58-
};
60+
bool busy;
61+
Condition cond;
62+
};
5963

60-
Mutex sLock;
64+
static Mutex sLock;
65+
static SortedVector <audiorecord_callback_cookie *> sAudioRecordCallBackCookies;
6166

6267
// ----------------------------------------------------------------------------
6368

@@ -87,13 +92,21 @@ jint android_media_translateRecorderErrorCode(int code) {
8792

8893
// ----------------------------------------------------------------------------
8994
static void recorderCallback(int event, void* user, void *info) {
95+
96+
audiorecord_callback_cookie *callbackInfo = (audiorecord_callback_cookie *)user;
97+
{
98+
Mutex::Autolock l(sLock);
99+
if (sAudioRecordCallBackCookies.indexOf(callbackInfo) < 0) {
100+
return;
101+
}
102+
callbackInfo->busy = true;
103+
}
90104
if (event == AudioRecord::EVENT_MORE_DATA) {
91105
// set size to 0 to signal we're not using the callback to read more data
92106
AudioRecord::Buffer* pBuff = (AudioRecord::Buffer*)info;
93107
pBuff->size = 0;
94108

95109
} else if (event == AudioRecord::EVENT_MARKER) {
96-
audiorecord_callback_cookie *callbackInfo = (audiorecord_callback_cookie *)user;
97110
JNIEnv *env = AndroidRuntime::getJNIEnv();
98111
if (user && env) {
99112
env->CallStaticVoidMethod(
@@ -107,7 +120,6 @@ static void recorderCallback(int event, void* user, void *info) {
107120
}
108121

109122
} else if (event == AudioRecord::EVENT_NEW_POS) {
110-
audiorecord_callback_cookie *callbackInfo = (audiorecord_callback_cookie *)user;
111123
JNIEnv *env = AndroidRuntime::getJNIEnv();
112124
if (user && env) {
113125
env->CallStaticVoidMethod(
@@ -120,8 +132,36 @@ static void recorderCallback(int event, void* user, void *info) {
120132
}
121133
}
122134
}
135+
{
136+
Mutex::Autolock l(sLock);
137+
callbackInfo->busy = false;
138+
callbackInfo->cond.broadcast();
139+
}
123140
}
124141

142+
// ----------------------------------------------------------------------------
143+
static sp<AudioRecord> getAudioRecord(JNIEnv* env, jobject thiz)
144+
{
145+
Mutex::Autolock l(sLock);
146+
AudioRecord* const ar =
147+
(AudioRecord*)env->GetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
148+
return sp<AudioRecord>(ar);
149+
}
150+
151+
static sp<AudioRecord> setAudioRecord(JNIEnv* env, jobject thiz, const sp<AudioRecord>& ar)
152+
{
153+
Mutex::Autolock l(sLock);
154+
sp<AudioRecord> old =
155+
(AudioRecord*)env->GetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
156+
if (ar.get()) {
157+
ar->incStrong(thiz);
158+
}
159+
if (old != 0) {
160+
old->decStrong(thiz);
161+
}
162+
env->SetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj, (int)ar.get());
163+
return old;
164+
}
125165

126166
// ----------------------------------------------------------------------------
127167
static int
@@ -162,6 +202,12 @@ android_media_AudioRecord_setup(JNIEnv *env, jobject thiz, jobject weak_this,
162202
return AUDIORECORD_ERROR_SETUP_INVALIDSOURCE;
163203
}
164204

205+
jclass clazz = env->GetObjectClass(thiz);
206+
if (clazz == NULL) {
207+
ALOGE("Can't find %s when setting up callback.", kClassPathName);
208+
return AUDIORECORD_ERROR_SETUP_NATIVEINITFAILED;
209+
}
210+
165211
if (jSession == NULL) {
166212
ALOGE("Error creating AudioRecord: invalid session ID pointer");
167213
return AUDIORECORD_ERROR;
@@ -176,27 +222,20 @@ android_media_AudioRecord_setup(JNIEnv *env, jobject thiz, jobject weak_this,
176222
env->ReleasePrimitiveArrayCritical(jSession, nSession, 0);
177223
nSession = NULL;
178224

179-
audiorecord_callback_cookie *lpCallbackData = NULL;
180-
AudioRecord* lpRecorder = NULL;
181-
182225
// create an uninitialized AudioRecord object
183-
lpRecorder = new AudioRecord();
184-
if (lpRecorder == NULL) {
226+
sp<AudioRecord> lpRecorder = new AudioRecord();
227+
if (lpRecorder == NULL) {
185228
ALOGE("Error creating AudioRecord instance.");
186229
return AUDIORECORD_ERROR_SETUP_NATIVEINITFAILED;
187230
}
188231

189232
// create the callback information:
190233
// this data will be passed with every AudioRecord callback
191-
jclass clazz = env->GetObjectClass(thiz);
192-
if (clazz == NULL) {
193-
ALOGE("Can't find %s when setting up callback.", kClassPathName);
194-
goto native_track_failure;
195-
}
196-
lpCallbackData = new audiorecord_callback_cookie;
234+
audiorecord_callback_cookie *lpCallbackData = new audiorecord_callback_cookie;
197235
lpCallbackData->audioRecord_class = (jclass)env->NewGlobalRef(clazz);
198236
// we use a weak reference so the AudioRecord object can be garbage collected.
199237
lpCallbackData->audioRecord_ref = env->NewGlobalRef(weak_this);
238+
lpCallbackData->busy = false;
200239

201240
lpRecorder->set((audio_source_t) source,
202241
sampleRateInHertz,
@@ -225,9 +264,13 @@ android_media_AudioRecord_setup(JNIEnv *env, jobject thiz, jobject weak_this,
225264
env->ReleasePrimitiveArrayCritical(jSession, nSession, 0);
226265
nSession = NULL;
227266

267+
{ // scope for the lock
268+
Mutex::Autolock l(sLock);
269+
sAudioRecordCallBackCookies.add(lpCallbackData);
270+
}
228271
// save our newly created C++ AudioRecord in the "nativeRecorderInJavaObj" field
229272
// of the Java object
230-
env->SetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj, (int)lpRecorder);
273+
setAudioRecord(env, thiz, lpRecorder);
231274

232275
// save our newly created callback information in the "nativeCallbackCookie" field
233276
// of the Java object (in mNativeCallbackCookie) so we can free the memory in finalize()
@@ -240,11 +283,6 @@ android_media_AudioRecord_setup(JNIEnv *env, jobject thiz, jobject weak_this,
240283
env->DeleteGlobalRef(lpCallbackData->audioRecord_class);
241284
env->DeleteGlobalRef(lpCallbackData->audioRecord_ref);
242285
delete lpCallbackData;
243-
244-
native_track_failure:
245-
delete lpRecorder;
246-
247-
env->SetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj, 0);
248286
env->SetIntField(thiz, javaAudioRecordFields.nativeCallbackCookie, 0);
249287

250288
return AUDIORECORD_ERROR_SETUP_NATIVEINITFAILED;
@@ -256,8 +294,7 @@ android_media_AudioRecord_setup(JNIEnv *env, jobject thiz, jobject weak_this,
256294
static int
257295
android_media_AudioRecord_start(JNIEnv *env, jobject thiz, jint event, jint triggerSession)
258296
{
259-
AudioRecord *lpRecorder =
260-
(AudioRecord *)env->GetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
297+
sp<AudioRecord> lpRecorder = getAudioRecord(env, thiz);
261298
if (lpRecorder == NULL ) {
262299
jniThrowException(env, "java/lang/IllegalStateException", NULL);
263300
return AUDIORECORD_ERROR;
@@ -272,8 +309,7 @@ android_media_AudioRecord_start(JNIEnv *env, jobject thiz, jint event, jint trig
272309
static void
273310
android_media_AudioRecord_stop(JNIEnv *env, jobject thiz)
274311
{
275-
AudioRecord *lpRecorder =
276-
(AudioRecord *)env->GetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
312+
sp<AudioRecord> lpRecorder = getAudioRecord(env, thiz);
277313
if (lpRecorder == NULL ) {
278314
jniThrowException(env, "java/lang/IllegalStateException", NULL);
279315
return;
@@ -285,30 +321,35 @@ android_media_AudioRecord_stop(JNIEnv *env, jobject thiz)
285321

286322

287323
// ----------------------------------------------------------------------------
324+
325+
#define CALLBACK_COND_WAIT_TIMEOUT_MS 1000
288326
static void android_media_AudioRecord_release(JNIEnv *env, jobject thiz) {
327+
sp<AudioRecord> lpRecorder = setAudioRecord(env, thiz, 0);
328+
if (lpRecorder == NULL) {
329+
return;
330+
}
331+
ALOGV("About to delete lpRecorder: %x\n", (int)lpRecorder.get());
332+
lpRecorder->stop();
289333

290-
// serialize access. Ugly, but functional.
291-
Mutex::Autolock lock(&sLock);
292-
AudioRecord *lpRecorder =
293-
(AudioRecord *)env->GetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
294334
audiorecord_callback_cookie *lpCookie = (audiorecord_callback_cookie *)env->GetIntField(
295335
thiz, javaAudioRecordFields.nativeCallbackCookie);
296336

297337
// reset the native resources in the Java object so any attempt to access
298338
// them after a call to release fails.
299-
env->SetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj, 0);
300339
env->SetIntField(thiz, javaAudioRecordFields.nativeCallbackCookie, 0);
301340

302-
// delete the AudioRecord object
303-
if (lpRecorder) {
304-
ALOGV("About to delete lpRecorder: %x\n", (int)lpRecorder);
305-
lpRecorder->stop();
306-
delete lpRecorder;
307-
}
308-
309341
// delete the callback information
310342
if (lpCookie) {
343+
Mutex::Autolock l(sLock);
311344
ALOGV("deleting lpCookie: %x\n", (int)lpCookie);
345+
while (lpCookie->busy) {
346+
if (lpCookie->cond.waitRelative(sLock,
347+
milliseconds(CALLBACK_COND_WAIT_TIMEOUT_MS)) !=
348+
NO_ERROR) {
349+
break;
350+
}
351+
}
352+
sAudioRecordCallBackCookies.remove(lpCookie);
312353
env->DeleteGlobalRef(lpCookie->audioRecord_class);
313354
env->DeleteGlobalRef(lpCookie->audioRecord_ref);
314355
delete lpCookie;
@@ -327,11 +368,8 @@ static jint android_media_AudioRecord_readInByteArray(JNIEnv *env, jobject thiz
327368
jbyteArray javaAudioData,
328369
jint offsetInBytes, jint sizeInBytes) {
329370
jbyte* recordBuff = NULL;
330-
AudioRecord *lpRecorder = NULL;
331-
332371
// get the audio recorder from which we'll read new audio samples
333-
lpRecorder =
334-
(AudioRecord *)env->GetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
372+
sp<AudioRecord> lpRecorder = getAudioRecord(env, thiz);
335373
if (lpRecorder == NULL) {
336374
ALOGE("Unable to retrieve AudioRecord object, can't record");
337375
return 0;
@@ -378,12 +416,8 @@ static jint android_media_AudioRecord_readInShortArray(JNIEnv *env, jobject thi
378416
// ----------------------------------------------------------------------------
379417
static jint android_media_AudioRecord_readInDirectBuffer(JNIEnv *env, jobject thiz,
380418
jobject jBuffer, jint sizeInBytes) {
381-
AudioRecord *lpRecorder = NULL;
382-
//ALOGV("Entering android_media_AudioRecord_readInBuffer");
383-
384419
// get the audio recorder from which we'll read new audio samples
385-
lpRecorder =
386-
(AudioRecord *)env->GetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
420+
sp<AudioRecord> lpRecorder = getAudioRecord(env, thiz);
387421
if (lpRecorder==NULL)
388422
return 0;
389423

@@ -411,71 +445,60 @@ static jint android_media_AudioRecord_readInDirectBuffer(JNIEnv *env, jobject t
411445
static jint android_media_AudioRecord_set_marker_pos(JNIEnv *env, jobject thiz,
412446
jint markerPos) {
413447

414-
AudioRecord *lpRecorder = (AudioRecord *)env->GetIntField(
415-
thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
416-
417-
if (lpRecorder) {
418-
return
419-
android_media_translateRecorderErrorCode( lpRecorder->setMarkerPosition(markerPos) );
420-
} else {
448+
sp<AudioRecord> lpRecorder = getAudioRecord(env, thiz);
449+
if (lpRecorder == NULL) {
421450
jniThrowException(env, "java/lang/IllegalStateException",
422451
"Unable to retrieve AudioRecord pointer for setMarkerPosition()");
423452
return AUDIORECORD_ERROR;
424453
}
454+
return android_media_translateRecorderErrorCode( lpRecorder->setMarkerPosition(markerPos) );
425455
}
426456

427457

428458
// ----------------------------------------------------------------------------
429459
static jint android_media_AudioRecord_get_marker_pos(JNIEnv *env, jobject thiz) {
430460

431-
AudioRecord *lpRecorder = (AudioRecord *)env->GetIntField(
432-
thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
461+
sp<AudioRecord> lpRecorder = getAudioRecord(env, thiz);
433462
uint32_t markerPos = 0;
434463

435-
if (lpRecorder) {
436-
lpRecorder->getMarkerPosition(&markerPos);
437-
return (jint)markerPos;
438-
} else {
464+
if (lpRecorder == NULL) {
439465
jniThrowException(env, "java/lang/IllegalStateException",
440466
"Unable to retrieve AudioRecord pointer for getMarkerPosition()");
441467
return AUDIORECORD_ERROR;
442468
}
469+
lpRecorder->getMarkerPosition(&markerPos);
470+
return (jint)markerPos;
443471
}
444472

445473

446474
// ----------------------------------------------------------------------------
447475
static jint android_media_AudioRecord_set_pos_update_period(JNIEnv *env, jobject thiz,
448476
jint period) {
449477

450-
AudioRecord *lpRecorder = (AudioRecord *)env->GetIntField(
451-
thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
478+
sp<AudioRecord> lpRecorder = getAudioRecord(env, thiz);
452479

453-
if (lpRecorder) {
454-
return
455-
android_media_translateRecorderErrorCode( lpRecorder->setPositionUpdatePeriod(period) );
456-
} else {
480+
if (lpRecorder == NULL) {
457481
jniThrowException(env, "java/lang/IllegalStateException",
458482
"Unable to retrieve AudioRecord pointer for setPositionUpdatePeriod()");
459483
return AUDIORECORD_ERROR;
460484
}
485+
return android_media_translateRecorderErrorCode( lpRecorder->setPositionUpdatePeriod(period) );
461486
}
462487

463488

464489
// ----------------------------------------------------------------------------
465490
static jint android_media_AudioRecord_get_pos_update_period(JNIEnv *env, jobject thiz) {
466491

467-
AudioRecord *lpRecorder = (AudioRecord *)env->GetIntField(
468-
thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
492+
sp<AudioRecord> lpRecorder = getAudioRecord(env, thiz);
469493
uint32_t period = 0;
470494

471-
if (lpRecorder) {
472-
lpRecorder->getPositionUpdatePeriod(&period);
473-
return (jint)period;
474-
} else {
495+
if (lpRecorder == NULL) {
475496
jniThrowException(env, "java/lang/IllegalStateException",
476497
"Unable to retrieve AudioRecord pointer for getPositionUpdatePeriod()");
477498
return AUDIORECORD_ERROR;
478499
}
500+
lpRecorder->getPositionUpdatePeriod(&period);
501+
return (jint)period;
479502
}
480503

481504

@@ -486,7 +509,8 @@ static jint android_media_AudioRecord_get_pos_update_period(JNIEnv *env, jobjec
486509
static jint android_media_AudioRecord_get_min_buff_size(JNIEnv *env, jobject thiz,
487510
jint sampleRateInHertz, jint nbChannels, jint audioFormat) {
488511

489-
ALOGV(">> android_media_AudioRecord_get_min_buff_size(%d, %d, %d)", sampleRateInHertz, nbChannels, audioFormat);
512+
ALOGV(">> android_media_AudioRecord_get_min_buff_size(%d, %d, %d)",
513+
sampleRateInHertz, nbChannels, audioFormat);
490514

491515
int frameCount = 0;
492516
status_t result = AudioRecord::getMinFrameCount(&frameCount,

0 commit comments

Comments
 (0)