Skip to content

Commit

Permalink
DO NOT MERGE - improve audio effect framwework thread safety
Browse files Browse the repository at this point in the history
- Reorganize handle effect creation code to make sure the effect engine
is created with both thread and effect chain mutex held.
- Reorganize handle disconnect code to make sure the effect engine
is released with both thread and effect chain mutex held.
- Protect IEffect interface methods in EffectHande with a Mutex.
- Only pin effect if the session was acquired first.
- Do not use strong pointer to EffectModule in EffectHandles:
only the EffectChain has a single strong reference to the EffectModule.

Bug: 32707507
Change-Id: Ia1098cba2cd32cc2d1c9dfdff4adc2388dfed80e
(cherry picked from commit b378b73)
  • Loading branch information
Eric Laurent authored and andi34 committed Mar 24, 2017
1 parent 30177e5 commit e27917e
Show file tree
Hide file tree
Showing 6 changed files with 212 additions and 99 deletions.
21 changes: 17 additions & 4 deletions services/audioflinger/AudioFlinger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1105,7 +1105,7 @@ void AudioFlinger::removeNotificationClient(pid_t pid)
ALOGV("%d died, releasing its sessions", pid);
size_t num = mAudioSessionRefs.size();
bool removed = false;
for (size_t i = 0; i< num; ) {
for (size_t i = 0; i < num; ) {
AudioSessionRef *ref = mAudioSessionRefs.itemAt(i);
ALOGV(" pid %d @ %d", ref->mPid, i);
if (ref->mPid == pid) {
Expand Down Expand Up @@ -1886,7 +1886,7 @@ void AudioFlinger::acquireAudioSessionId(int audioSession)
}

size_t num = mAudioSessionRefs.size();
for (size_t i = 0; i< num; i++) {
for (size_t i = 0; i < num; i++) {
AudioSessionRef *ref = mAudioSessionRefs.editItemAt(i);
if (ref->mSessionid == audioSession && ref->mPid == caller) {
ref->mCnt++;
Expand All @@ -1904,7 +1904,7 @@ void AudioFlinger::releaseAudioSessionId(int audioSession)
pid_t caller = IPCThreadState::self()->getCallingPid();
ALOGV("releasing %d from %d", audioSession, caller);
size_t num = mAudioSessionRefs.size();
for (size_t i = 0; i< num; i++) {
for (size_t i = 0; i < num; i++) {
AudioSessionRef *ref = mAudioSessionRefs.itemAt(i);
if (ref->mSessionid == audioSession && ref->mPid == caller) {
ref->mCnt--;
Expand All @@ -1922,6 +1922,18 @@ void AudioFlinger::releaseAudioSessionId(int audioSession)
ALOGW_IF(caller != getpid_cached, "session id %d not found for pid %d", audioSession, caller);
}

bool AudioFlinger::isSessionAcquired_l(int audioSession)
{
size_t num = mAudioSessionRefs.size();
for (size_t i = 0; i < num; i++) {
AudioSessionRef *ref = mAudioSessionRefs.itemAt(i);
if (ref->mSessionid == audioSession) {
return true;
}
}
return false;
}

void AudioFlinger::purgeStaleEffects_l() {

ALOGV("purging stale effects");
Expand Down Expand Up @@ -2254,8 +2266,9 @@ sp<IEffect> AudioFlinger::createEffect(
sp<Client> client = registerPid_l(pid);

// create effect on selected output thread
bool pinned = (sessionId > AUDIO_SESSION_OUTPUT_MIX) && isSessionAcquired_l(sessionId);
handle = thread->createEffect_l(client, effectClient, priority, sessionId,
&desc, enabled, &lStatus);
&desc, enabled, &lStatus, pinned);
if (handle != 0 && id != NULL) {
*id = handle->id();
}
Expand Down
2 changes: 1 addition & 1 deletion services/audioflinger/AudioFlinger.h
Original file line number Diff line number Diff line change
Expand Up @@ -467,9 +467,9 @@ class AudioFlinger :

void removeClient_l(pid_t pid);
void removeNotificationClient(pid_t pid);

bool isNonOffloadableGlobalEffectEnabled_l();
void onNonOffloadableGlobalEffectEnable();
bool isSessionAcquired_l(int audioSession);

class AudioHwDevice {
public:
Expand Down
Loading

0 comments on commit e27917e

Please sign in to comment.