From 685c9ce3bd5782f52f2ca52d944ccd26933e79f2 Mon Sep 17 00:00:00 2001 From: Glenn Kasten Date: Fri, 20 Jan 2012 13:32:16 -0800 Subject: [PATCH] Avoid wp<>::unsafe_get() with a few exceptions Avoid using wp<>::unsafe_get() except in a log, and other specific cases when it's known to be safe. Use more specific subclass types for parameters to avoid down-casts. When a constructor or method parameter is "this" of an object that is currently being constructed, it's better to use a raw pointer rather than either sp<> or wp<>. Using the raw pointer is safe, provided either: - it is "this" of an object being constructed (which has sp<> refcount of 0), - or the caller already holds an sp<> The raw pointer is simpler and faster, and it avoids the problem of the sp<> reference count being incremented and then decremented to zero on scope exit, which would cause the object's destructor to run while the object is still being constructed. Also removed some dead code per a review comment. Change-Id: I7375f64da3aec11b928c33cb01faff186252ef5e --- services/audioflinger/AudioFlinger.cpp | 48 +++++++++----------------- services/audioflinger/AudioFlinger.h | 15 ++++---- 2 files changed, 25 insertions(+), 38 deletions(-) diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp index 2e2834c3c7f60..5af163a8f6d5a 100644 --- a/services/audioflinger/AudioFlinger.cpp +++ b/services/audioflinger/AudioFlinger.cpp @@ -3231,7 +3231,7 @@ void AudioFlinger::DuplicatingThread::addOutputTrack(MixerThread *thread) { // FIXME explain this formula int frameCount = (3 * mFrameCount * mSampleRate) / thread->sampleRate(); - OutputTrack *outputTrack = new OutputTrack((ThreadBase *)thread, + OutputTrack *outputTrack = new OutputTrack(thread, this, mSampleRate, mFormat, @@ -3300,7 +3300,7 @@ uint32_t AudioFlinger::DuplicatingThread::activeSleepTimeUs() // TrackBase constructor must be called with AudioFlinger::mLock held AudioFlinger::ThreadBase::TrackBase::TrackBase( - const wp& thread, + ThreadBase *thread, const sp& client, uint32_t sampleRate, audio_format_t format, @@ -3456,7 +3456,7 @@ void* AudioFlinger::ThreadBase::TrackBase::getBuffer(uint32_t offset, uint32_t f // Track constructor must be called with AudioFlinger::mLock and ThreadBase::mLock held AudioFlinger::PlaybackThread::Track::Track( - const wp& thread, + PlaybackThread *thread, const sp& client, audio_stream_type_t streamType, uint32_t sampleRate, @@ -3470,11 +3470,9 @@ AudioFlinger::PlaybackThread::Track::Track( mAuxEffectId(0), mHasVolumeController(false) { if (mCblk != NULL) { - sp baseThread = thread.promote(); - if (baseThread != 0) { - PlaybackThread *playbackThread = (PlaybackThread *)baseThread.get(); - mName = playbackThread->getTrackName_l(); - mMainBuffer = playbackThread->mixBuffer(); + if (thread != NULL) { + mName = thread->getTrackName_l(); + mMainBuffer = thread->mixBuffer(); } ALOGV("Track constructor name %d, calling pid %d", mName, IPCThreadState::self()->getCallingPid()); if (mName < 0) { @@ -3760,7 +3758,7 @@ void AudioFlinger::PlaybackThread::Track::setAuxBuffer(int EffectId, int32_t *bu sp AudioFlinger::PlaybackThread::TimedTrack::create( - const wp& thread, + PlaybackThread *thread, const sp& client, audio_stream_type_t streamType, uint32_t sampleRate, @@ -3785,7 +3783,7 @@ AudioFlinger::PlaybackThread::TimedTrack::create( } AudioFlinger::PlaybackThread::TimedTrack::TimedTrack( - const wp& thread, + PlaybackThread *thread, const sp& client, audio_stream_type_t streamType, uint32_t sampleRate, @@ -3926,15 +3924,6 @@ status_t AudioFlinger::PlaybackThread::TimedTrack::getNextBuffer( return INVALID_OPERATION; } - // get ahold of the output stream that these samples will be written to - sp thread = mThread.promote(); - if (thread == NULL) { - buffer->raw = 0; - buffer->frameCount = 0; - return INVALID_OPERATION; - } - PlaybackThread* playbackThread = static_cast(thread.get()); - Mutex::Autolock _l(mTimedBufferQueueLock); while (true) { @@ -4160,7 +4149,7 @@ AudioFlinger::PlaybackThread::TimedTrack::TimedBuffer::TimedBuffer( // RecordTrack constructor must be called with AudioFlinger::mLock held AudioFlinger::RecordThread::RecordTrack::RecordTrack( - const wp& thread, + RecordThread *thread, const sp& client, uint32_t sampleRate, audio_format_t format, @@ -4273,17 +4262,16 @@ void AudioFlinger::RecordThread::RecordTrack::dump(char* buffer, size_t size) // ---------------------------------------------------------------------------- AudioFlinger::PlaybackThread::OutputTrack::OutputTrack( - const wp& thread, + PlaybackThread *playbackThread, DuplicatingThread *sourceThread, uint32_t sampleRate, audio_format_t format, uint32_t channelMask, int frameCount) - : Track(thread, NULL, AUDIO_STREAM_CNT, sampleRate, format, channelMask, frameCount, NULL, 0), + : Track(playbackThread, NULL, AUDIO_STREAM_CNT, sampleRate, format, channelMask, frameCount, NULL, 0), mActive(false), mSourceThread(sourceThread) { - PlaybackThread *playbackThread = (PlaybackThread *)thread.unsafe_get(); if (mCblk != NULL) { mCblk->flags |= CBLK_DIRECTION_OUT; mCblk->buffers = (char*)mCblk + sizeof(audio_track_cblk_t); @@ -6595,18 +6583,17 @@ size_t AudioFlinger::RecordThread::removeEffectChain_l(const sp& ch #undef LOG_TAG #define LOG_TAG "AudioFlinger::EffectModule" -AudioFlinger::EffectModule::EffectModule(const wp& wThread, +AudioFlinger::EffectModule::EffectModule(ThreadBase *thread, const wp& chain, effect_descriptor_t *desc, int id, int sessionId) - : mThread(wThread), mChain(chain), mId(id), mSessionId(sessionId), mEffectInterface(NULL), + : mThread(thread), mChain(chain), mId(id), mSessionId(sessionId), mEffectInterface(NULL), mStatus(NO_INIT), mState(IDLE), mSuspended(false) { ALOGV("Constructor %p", this); int lStatus; - sp thread = mThread.promote(); - if (thread == 0) { + if (thread == NULL) { return; } @@ -7576,15 +7563,14 @@ void AudioFlinger::EffectHandle::dump(char* buffer, size_t size) #undef LOG_TAG #define LOG_TAG "AudioFlinger::EffectChain" -AudioFlinger::EffectChain::EffectChain(const wp& wThread, +AudioFlinger::EffectChain::EffectChain(ThreadBase *thread, int sessionId) - : mThread(wThread), mSessionId(sessionId), mActiveTrackCnt(0), mTrackCnt(0), mTailBufferCount(0), + : mThread(thread), mSessionId(sessionId), mActiveTrackCnt(0), mTrackCnt(0), mTailBufferCount(0), mOwnInBuffer(false), mVolumeCtrlIdx(-1), mLeftVolume(UINT_MAX), mRightVolume(UINT_MAX), mNewLeftVolume(UINT_MAX), mNewRightVolume(UINT_MAX) { mStrategy = AudioSystem::getStrategyForStream(AUDIO_STREAM_MUSIC); - sp thread = mThread.promote(); - if (thread == 0) { + if (thread == NULL) { return; } mMaxTailBuffers = ((kProcessTailDurationMs * thread->sampleRate()) / 1000) / diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h index 50712cf1fa8aa..1a52de5b67d75 100644 --- a/services/audioflinger/AudioFlinger.h +++ b/services/audioflinger/AudioFlinger.h @@ -318,7 +318,7 @@ private: // The upper 16 bits are used for track-specific flags. }; - TrackBase(const wp& thread, + TrackBase(ThreadBase *thread, const sp& client, uint32_t sampleRate, audio_format_t format, @@ -591,7 +591,7 @@ private: // playback track class Track : public TrackBase { public: - Track( const wp& thread, + Track( PlaybackThread *thread, const sp& client, audio_stream_type_t streamType, uint32_t sampleRate, @@ -674,7 +674,7 @@ private: class TimedTrack : public Track { public: - static sp create(const wp& thread, + static sp create(PlaybackThread *thread, const sp& client, audio_stream_type_t streamType, uint32_t sampleRate, @@ -719,7 +719,7 @@ private: void trimTimedBufferQueue_l(); private: - TimedTrack(const wp& thread, + TimedTrack(PlaybackThread *thread, const sp& client, audio_stream_type_t streamType, uint32_t sampleRate, @@ -755,7 +755,7 @@ private: int16_t *mBuffer; }; - OutputTrack( const wp& thread, + OutputTrack(PlaybackThread *thread, DuplicatingThread *sourceThread, uint32_t sampleRate, audio_format_t format, @@ -1042,7 +1042,7 @@ private: // record track class RecordTrack : public TrackBase { public: - RecordTrack(const wp& thread, + RecordTrack(RecordThread *thread, const sp& client, uint32_t sampleRate, audio_format_t format, @@ -1168,7 +1168,7 @@ private: // the attached track(s) to accumulate their auxiliary channel. class EffectModule: public RefBase { public: - EffectModule(const wp& wThread, + EffectModule(ThreadBase *thread, const wp& chain, effect_descriptor_t *desc, int id, @@ -1353,6 +1353,7 @@ mutable Mutex mLock; // mutex for process, commands and handl class EffectChain: public RefBase { public: EffectChain(const wp& wThread, int sessionId); + EffectChain(ThreadBase *thread, int sessionId); virtual ~EffectChain(); // special key used for an entry in mSuspendedEffects keyed vector