diff options
| -rw-r--r-- | services/audioflinger/AudioFlinger.cpp | 102 | ||||
| -rw-r--r-- | services/audioflinger/AudioFlinger.h | 17 | ||||
| -rw-r--r-- | services/audioflinger/Effects.cpp | 4 | ||||
| -rw-r--r-- | services/audioflinger/Threads.cpp | 33 | ||||
| -rw-r--r-- | services/audioflinger/Threads.h | 19 | ||||
| -rw-r--r-- | services/audioflinger/Tracks.cpp | 4 |
6 files changed, 87 insertions, 92 deletions
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp index e256f32..11170c2 100644 --- a/services/audioflinger/AudioFlinger.cpp +++ b/services/audioflinger/AudioFlinger.cpp @@ -82,6 +82,7 @@ namespace android { static const char kDeadlockedString[] = "AudioFlinger may be deadlocked\n"; static const char kHardwareLockedString[] = "Hardware lock is taken\n"; +static const char kClientLockedString[] = "Client lock is taken\n"; nsecs_t AudioFlinger::mStandbyTimeInNsecs = kDefaultStandbyTimeInNsecs; @@ -382,7 +383,16 @@ status_t AudioFlinger::dump(int fd, const Vector<String16>& args) write(fd, result.string(), result.size()); } + bool clientLocked = dumpTryLock(mClientLock); + if (!clientLocked) { + String8 result(kClientLockedString); + write(fd, result.string(), result.size()); + } dumpClients(fd, args); + if (clientLocked) { + mClientLock.unlock(); + } + dumpInternals(fd, args); // dump playback threads @@ -426,8 +436,9 @@ status_t AudioFlinger::dump(int fd, const Vector<String16>& args) return NO_ERROR; } -sp<AudioFlinger::Client> AudioFlinger::registerPid_l(pid_t pid) +sp<AudioFlinger::Client> AudioFlinger::registerPid(pid_t pid) { + Mutex::Autolock _cl(mClientLock); // If pid is already in the mClients wp<> map, then use that entry // (for which promote() is always != 0), otherwise create a new entry and Client. sp<Client> client = mClients.valueFor(pid).promote(); @@ -564,7 +575,7 @@ sp<IAudioTrack> AudioFlinger::createTrack( } pid_t pid = IPCThreadState::self()->getCallingPid(); - client = registerPid_l(pid); + client = registerPid(pid); PlaybackThread *effectThread = NULL; if (sessionId != NULL && *sessionId != AUDIO_SESSION_ALLOCATE) { @@ -623,7 +634,8 @@ sp<IAudioTrack> AudioFlinger::createTrack( if (lStatus != NO_ERROR) { // remove local strong reference to Client before deleting the Track so that the - // Client destructor is called by the TrackBase destructor with mLock held + // Client destructor is called by the TrackBase destructor with mClientLock held + Mutex::Autolock _cl(mClientLock); client.clear(); track.clear(); goto Exit; @@ -1140,21 +1152,29 @@ status_t AudioFlinger::getRenderPosition(uint32_t *halFrames, uint32_t *dspFrame void AudioFlinger::registerClient(const sp<IAudioFlingerClient>& client) { - Mutex::Autolock _l(mLock); + bool clientAdded = false; + { + Mutex::Autolock _cl(mClientLock); - pid_t pid = IPCThreadState::self()->getCallingPid(); - if (mNotificationClients.indexOfKey(pid) < 0) { - sp<NotificationClient> notificationClient = new NotificationClient(this, - client, - pid); - ALOGV("registerClient() client %p, pid %d", notificationClient.get(), pid); + pid_t pid = IPCThreadState::self()->getCallingPid(); + if (mNotificationClients.indexOfKey(pid) < 0) { + sp<NotificationClient> notificationClient = new NotificationClient(this, + client, + pid); + ALOGV("registerClient() client %p, pid %d", notificationClient.get(), pid); - mNotificationClients.add(pid, notificationClient); + mNotificationClients.add(pid, notificationClient); - sp<IBinder> binder = client->asBinder(); - binder->linkToDeath(notificationClient); + sp<IBinder> binder = client->asBinder(); + binder->linkToDeath(notificationClient); + clientAdded = true; + } + } + // mClientLock should not be held here because ThreadBase::sendIoConfigEvent() will lock the + // ThreadBase mutex and teh locknig order is ThreadBase::mLock then AudioFlinger::mClientLock. + if (clientAdded) { // the config change is always sent from playback or record threads to avoid deadlock // with AudioSystem::gLock for (size_t i = 0; i < mPlaybackThreads.size(); i++) { @@ -1170,8 +1190,10 @@ void AudioFlinger::registerClient(const sp<IAudioFlingerClient>& client) void AudioFlinger::removeNotificationClient(pid_t pid) { Mutex::Autolock _l(mLock); - - mNotificationClients.removeItem(pid); + { + Mutex::Autolock _cl(mClientLock); + mNotificationClients.removeItem(pid); + } ALOGV("%d died, releasing its sessions", pid); size_t num = mAudioSessionRefs.size(); @@ -1194,22 +1216,18 @@ void AudioFlinger::removeNotificationClient(pid_t pid) } } -// audioConfigChanged_l() must be called with AudioFlinger::mLock held -void AudioFlinger::audioConfigChanged_l( - const DefaultKeyedVector< pid_t,sp<NotificationClient> >& notificationClients, - int event, - audio_io_handle_t ioHandle, - const void *param2) +void AudioFlinger::audioConfigChanged(int event, audio_io_handle_t ioHandle, const void *param2) { - size_t size = notificationClients.size(); + Mutex::Autolock _l(mClientLock); + size_t size = mNotificationClients.size(); for (size_t i = 0; i < size; i++) { - notificationClients.valueAt(i)->audioFlingerClient()->ioConfigChanged(event, + mNotificationClients.valueAt(i)->audioFlingerClient()->ioConfigChanged(event, ioHandle, param2); } } -// removeClient_l() must be called with AudioFlinger::mLock held +// removeClient_l() must be called with AudioFlinger::mClientLock held void AudioFlinger::removeClient_l(pid_t pid) { ALOGV("removeClient_l() pid %d, calling pid %d", pid, @@ -1247,7 +1265,7 @@ AudioFlinger::Client::Client(const sp<AudioFlinger>& audioFlinger, pid_t pid) // 1 MB of address space is good for 32 tracks, 8 buffers each, 4 KB/buffer } -// Client destructor must be called with AudioFlinger::mLock held +// Client destructor must be called with AudioFlinger::mClientLock held AudioFlinger::Client::~Client() { mAudioFlinger->removeClient_l(mPid); @@ -1377,7 +1395,7 @@ sp<IAudioRecord> AudioFlinger::openRecord( } pid_t pid = IPCThreadState::self()->getCallingPid(); - client = registerPid_l(pid); + client = registerPid(pid); if (sessionId != NULL && *sessionId != AUDIO_SESSION_ALLOCATE) { lSessionId = *sessionId; @@ -1400,7 +1418,8 @@ sp<IAudioRecord> AudioFlinger::openRecord( if (lStatus != NO_ERROR) { // remove local strong reference to Client before deleting the RecordTrack so that the - // Client destructor is called by the TrackBase destructor with mLock held + // Client destructor is called by the TrackBase destructor with mClientLock held + Mutex::Autolock _cl(mClientLock); client.clear(); recordTrack.clear(); goto Exit; @@ -1638,7 +1657,7 @@ audio_io_handle_t AudioFlinger::openOutput(audio_module_handle_t module, } // notify client processes of the new output creation - thread->audioConfigChanged_l(mNotificationClients, AudioSystem::OUTPUT_OPENED); + thread->audioConfigChanged(AudioSystem::OUTPUT_OPENED); // the first primary output opened designates the primary hw device if ((mPrimaryHardwareDev == NULL) && (flags & AUDIO_OUTPUT_FLAG_PRIMARY)) { @@ -1674,7 +1693,7 @@ audio_io_handle_t AudioFlinger::openDuplicateOutput(audio_io_handle_t output1, thread->addOutputTrack(thread2); mPlaybackThreads.add(id, thread); // notify client processes of the new output creation - thread->audioConfigChanged_l(mNotificationClients, AudioSystem::OUTPUT_OPENED); + thread->audioConfigChanged(AudioSystem::OUTPUT_OPENED); return id; } @@ -1724,7 +1743,7 @@ status_t AudioFlinger::closeOutput_nonvirtual(audio_io_handle_t output) } } } - audioConfigChanged_l(mNotificationClients, AudioSystem::OUTPUT_CLOSED, output, NULL); + audioConfigChanged(AudioSystem::OUTPUT_CLOSED, output, NULL); } thread->exit(); // The thread entity (active unit of execution) is no longer running here, @@ -1904,7 +1923,7 @@ audio_io_handle_t AudioFlinger::openInput(audio_module_handle_t module, } // notify client processes of the new input creation - thread->audioConfigChanged_l(mNotificationClients, AudioSystem::INPUT_OPENED); + thread->audioConfigChanged(AudioSystem::INPUT_OPENED); return id; } @@ -1929,7 +1948,7 @@ status_t AudioFlinger::closeInput_nonvirtual(audio_io_handle_t input) } ALOGV("closeInput() %d", input); - audioConfigChanged_l(mNotificationClients, AudioSystem::INPUT_CLOSED, input, NULL); + audioConfigChanged(AudioSystem::INPUT_CLOSED, input, NULL); mRecordThreads.removeItem(input); } thread->exit(); @@ -1973,13 +1992,16 @@ void AudioFlinger::acquireAudioSessionId(int audioSession, pid_t pid) caller = pid; } - // Ignore requests received from processes not known as notification client. The request - // is likely proxied by mediaserver (e.g CameraService) and releaseAudioSessionId() can be - // called from a different pid leaving a stale session reference. Also we don't know how - // to clear this reference if the client process dies. - if (mNotificationClients.indexOfKey(caller) < 0) { - ALOGW("acquireAudioSessionId() unknown client %d for session %d", caller, audioSession); - return; + { + Mutex::Autolock _cl(mClientLock); + // Ignore requests received from processes not known as notification client. The request + // is likely proxied by mediaserver (e.g CameraService) and releaseAudioSessionId() can be + // called from a different pid leaving a stale session reference. Also we don't know how + // to clear this reference if the client process dies. + if (mNotificationClients.indexOfKey(caller) < 0) { + ALOGW("acquireAudioSessionId() unknown client %d for session %d", caller, audioSession); + return; + } } size_t num = mAudioSessionRefs.size(); @@ -2348,7 +2370,7 @@ sp<IEffect> AudioFlinger::createEffect( } } - sp<Client> client = registerPid_l(pid); + sp<Client> client = registerPid(pid); // create effect on selected output thread handle = thread->createEffect_l(client, effectClient, priority, sessionId, diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h index 894bd35..c1d4c08 100644 --- a/services/audioflinger/AudioFlinger.h +++ b/services/audioflinger/AudioFlinger.h @@ -453,11 +453,7 @@ private: // no range check, doesn't check per-thread stream volume, AudioFlinger::mLock held float streamVolume_l(audio_stream_type_t stream) const { return mStreamTypes[stream].volume; } - void audioConfigChanged_l(const DefaultKeyedVector< pid_t,sp<NotificationClient> >& - notificationClients, - int event, - audio_io_handle_t ioHandle, - const void *param2); + void audioConfigChanged(int event, audio_io_handle_t ioHandle, const void *param2); // Allocate an audio_io_handle_t, session ID, effect ID, or audio_module_handle_t. // They all share the same ID space, but the namespaces are actually independent @@ -482,8 +478,6 @@ private: void removeClient_l(pid_t pid); void removeNotificationClient(pid_t pid); - DefaultKeyedVector< pid_t,sp<NotificationClient> > notificationClients() { - Mutex::Autolock _l(mLock); return mNotificationClients; } bool isNonOffloadableGlobalEffectEnabled_l(); void onNonOffloadableGlobalEffectEnable(); @@ -553,7 +547,11 @@ private: }; mutable Mutex mLock; - + // protects mClients and mNotificationClients. + // must be locked after mLock and ThreadBase::mLock if both must be locked + // avoids acquiring AudioFlinger::mLock from inside thread loop. + mutable Mutex mClientLock; + // protected by mClientLock DefaultKeyedVector< pid_t, wp<Client> > mClients; // see ~Client() mutable Mutex mHardwareLock; @@ -602,6 +600,7 @@ private: DefaultKeyedVector< audio_io_handle_t, sp<RecordThread> > mRecordThreads; + // protected by mClientLock DefaultKeyedVector< pid_t, sp<NotificationClient> > mNotificationClients; volatile int32_t mNextUniqueId; // updated by android_atomic_inc @@ -622,7 +621,7 @@ private: // to be created private: - sp<Client> registerPid_l(pid_t pid); // always returns non-0 + sp<Client> registerPid(pid_t pid); // always returns non-0 // for use from destructor status_t closeOutput_nonvirtual(audio_io_handle_t output); diff --git a/services/audioflinger/Effects.cpp b/services/audioflinger/Effects.cpp index 29b56db..77aca00 100644 --- a/services/audioflinger/Effects.cpp +++ b/services/audioflinger/Effects.cpp @@ -1162,8 +1162,8 @@ void AudioFlinger::EffectHandle::disconnect(bool unpinIfLast) mCblk->~effect_param_cblk_t(); // destroy our shared-structure. } mCblkMemory.clear(); // free the shared memory before releasing the heap it belongs to - // Client destructor must run with AudioFlinger mutex locked - Mutex::Autolock _l(mClient->audioFlinger()->mLock); + // Client destructor must run with AudioFlinger client mutex locked + Mutex::Autolock _l(mClient->audioFlinger()->mClientLock); mClient.clear(); } } diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp index 470b018..8243a8b 100644 --- a/services/audioflinger/Threads.cpp +++ b/services/audioflinger/Threads.cpp @@ -401,8 +401,7 @@ status_t AudioFlinger::ThreadBase::sendSetParameterConfigEvent_l(const String8& } // post condition: mConfigEvents.isEmpty() -void AudioFlinger::ThreadBase::processConfigEvents_l( - const DefaultKeyedVector< pid_t,sp<NotificationClient> >& notificationClients) +void AudioFlinger::ThreadBase::processConfigEvents_l() { bool configChanged = false; @@ -423,7 +422,7 @@ void AudioFlinger::ThreadBase::processConfigEvents_l( } break; case CFG_EVENT_IO: { IoConfigEventData *data = (IoConfigEventData *)event->mData.get(); - audioConfigChanged_l(notificationClients, data->mEvent, data->mParam); + audioConfigChanged(data->mEvent, data->mParam); } break; case CFG_EVENT_SET_PARAMETER: { SetParameterConfigEventData *data = (SetParameterConfigEventData *)event->mData.get(); @@ -1638,15 +1637,11 @@ String8 AudioFlinger::PlaybackThread::getParameters(const String8& keys) return out_s8; } -// audioConfigChanged_l() must be called with AudioFlinger::mLock held -void AudioFlinger::PlaybackThread::audioConfigChanged_l( - const DefaultKeyedVector< pid_t,sp<NotificationClient> >& notificationClients, - int event, - int param) { +void AudioFlinger::PlaybackThread::audioConfigChanged(int event, int param) { AudioSystem::OutputDescriptor desc; void *param2 = NULL; - ALOGV("PlaybackThread::audioConfigChanged_l, thread %p, event %d, param %d", this, event, + ALOGV("PlaybackThread::audioConfigChanged, thread %p, event %d, param %d", this, event, param); switch (event) { @@ -1667,7 +1662,7 @@ void AudioFlinger::PlaybackThread::audioConfigChanged_l( default: break; } - mAudioFlinger->audioConfigChanged_l(notificationClients, event, mId, param2); + mAudioFlinger->audioConfigChanged(event, mId, param2); } void AudioFlinger::PlaybackThread::writeCallback() @@ -2317,15 +2312,11 @@ bool AudioFlinger::PlaybackThread::threadLoop() Vector< sp<EffectChain> > effectChains; - DefaultKeyedVector< pid_t,sp<NotificationClient> > notificationClients = - mAudioFlinger->notificationClients(); - { // scope for mLock Mutex::Autolock _l(mLock); - processConfigEvents_l(notificationClients); - notificationClients.clear(); + processConfigEvents_l(); if (logString != NULL) { mNBLogWriter->logTimestamp(); @@ -4695,14 +4686,11 @@ reacquire_wakelock: // activeTracks accumulates a copy of a subset of mActiveTracks Vector< sp<RecordTrack> > activeTracks; - DefaultKeyedVector< pid_t,sp<NotificationClient> > notificationClients = - mAudioFlinger->notificationClients(); { // scope for mLock Mutex::Autolock _l(mLock); - processConfigEvents_l(notificationClients); - notificationClients.clear(); + processConfigEvents_l(); // check exitPending here because checkForNewParameters_l() and // checkForNewParameters_l() can temporarily release mLock @@ -5605,10 +5593,7 @@ String8 AudioFlinger::RecordThread::getParameters(const String8& keys) return out_s8; } -void AudioFlinger::RecordThread::audioConfigChanged_l( - const DefaultKeyedVector< pid_t,sp<NotificationClient> >& notificationClients, - int event, - int param __unused) { +void AudioFlinger::RecordThread::audioConfigChanged(int event, int param __unused) { AudioSystem::OutputDescriptor desc; const void *param2 = NULL; @@ -5627,7 +5612,7 @@ void AudioFlinger::RecordThread::audioConfigChanged_l( default: break; } - mAudioFlinger->audioConfigChanged_l(notificationClients, event, mId, param2); + mAudioFlinger->audioConfigChanged(event, mId, param2); } void AudioFlinger::RecordThread::readInputParameters_l() diff --git a/services/audioflinger/Threads.h b/services/audioflinger/Threads.h index 9578993..cc2b246 100644 --- a/services/audioflinger/Threads.h +++ b/services/audioflinger/Threads.h @@ -200,10 +200,7 @@ public: status_t& status) = 0; virtual status_t setParameters(const String8& keyValuePairs); virtual String8 getParameters(const String8& keys) = 0; - virtual void audioConfigChanged_l( - const DefaultKeyedVector< pid_t,sp<NotificationClient> >& notificationClients, - int event, - int param = 0) = 0; + virtual void audioConfigChanged(int event, int param = 0) = 0; // sendConfigEvent_l() must be called with ThreadBase::mLock held // Can temporarily release the lock if waiting for a reply from // processConfigEvents_l(). @@ -212,8 +209,7 @@ public: void sendIoConfigEvent_l(int event, int param = 0); void sendPrioConfigEvent_l(pid_t pid, pid_t tid, int32_t prio); status_t sendSetParameterConfigEvent_l(const String8& keyValuePair); - void processConfigEvents_l( - const DefaultKeyedVector< pid_t,sp<NotificationClient> >& notificationClients); + void processConfigEvents_l(); virtual void cacheParameters_l() = 0; // see note at declaration of mStandby, mOutDevice and mInDevice @@ -502,10 +498,7 @@ public: { return android_atomic_acquire_load(&mSuspended) > 0; } virtual String8 getParameters(const String8& keys); - virtual void audioConfigChanged_l( - const DefaultKeyedVector< pid_t,sp<NotificationClient> >& notificationClients, - int event, - int param = 0); + virtual void audioConfigChanged(int event, int param = 0); status_t getRenderPosition(uint32_t *halFrames, uint32_t *dspFrames); // FIXME rename mixBuffer() to sinkBuffer() and remove int16_t* dependency. // Consider also removing and passing an explicit mMainBuffer initialization @@ -652,7 +645,6 @@ private: friend class AudioFlinger; // for numerous - PlaybackThread(const Client&); PlaybackThread& operator = (const PlaybackThread&); status_t addTrack_l(const sp<Track>& track); @@ -1037,10 +1029,7 @@ public: status_t& status); virtual void cacheParameters_l() {} virtual String8 getParameters(const String8& keys); - virtual void audioConfigChanged_l( - const DefaultKeyedVector< pid_t,sp<NotificationClient> >& notificationClients, - int event, - int param = 0); + virtual void audioConfigChanged(int event, int param = 0); void readInputParameters_l(); virtual uint32_t getInputFramesLost(); diff --git a/services/audioflinger/Tracks.cpp b/services/audioflinger/Tracks.cpp index d8f3423..88ead74 100644 --- a/services/audioflinger/Tracks.cpp +++ b/services/audioflinger/Tracks.cpp @@ -198,8 +198,8 @@ AudioFlinger::ThreadBase::TrackBase::~TrackBase() } mCblkMemory.clear(); // free the shared memory before releasing the heap it belongs to if (mClient != 0) { - // Client destructor must run with AudioFlinger mutex locked - Mutex::Autolock _l(mClient->audioFlinger()->mLock); + // Client destructor must run with AudioFlinger client mutex locked + Mutex::Autolock _l(mClient->audioFlinger()->mClientLock); // If the client's reference count drops to zero, the associated destructor // must run with AudioFlinger lock held. Thus the explicit clear() rather than // relying on the automatic clear() at end of scope. |