From 9fabbf8a8ae755c3c07637f8ca7677c63a1607bb Mon Sep 17 00:00:00 2001 From: Phil Burk Date: Thu, 3 Aug 2017 12:02:00 -0700 Subject: [PATCH] audioflinger: prevent crash in MmapThreadHandle destructor The mThread smart pointer was getting cleared while the thread was still in use. Bug: 64316921 Test: use input_monitor.cpp, see bug for repro steps Change-Id: I46095b4e67648208a2fb979696fc2d655432448b --- services/audioflinger/AudioFlinger.h | 2 +- services/audioflinger/Threads.cpp | 27 +++------------------------ 2 files changed, 4 insertions(+), 25 deletions(-) diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h index 9023b2d52f..63898a0fd6 100644 --- a/services/audioflinger/AudioFlinger.h +++ b/services/audioflinger/AudioFlinger.h @@ -604,7 +604,7 @@ private: virtual status_t standby(); private: - sp mThread; + const sp mThread; }; ThreadBase *checkThread_l(audio_io_handle_t ioHandle) const; diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp index 459e4fbbd6..8c4531a411 100644 --- a/services/audioflinger/Threads.cpp +++ b/services/audioflinger/Threads.cpp @@ -7503,34 +7503,22 @@ void AudioFlinger::RecordThread::getAudioPortConfig(struct audio_port_config *co AudioFlinger::MmapThreadHandle::MmapThreadHandle(const sp& thread) : mThread(thread) { + assert(thread != 0); // thread must start non-null and stay non-null } AudioFlinger::MmapThreadHandle::~MmapThreadHandle() { - MmapThread *thread = mThread.get(); - // clear our strong reference before disconnecting the thread: the last strong reference - // will be removed when closeInput/closeOutput is executed upon call from audio policy manager - // and the thread removed from mMMapThreads list causing the thread destruction. - mThread.clear(); - if (thread != nullptr) { - thread->disconnect(); - } + mThread->disconnect(); } status_t AudioFlinger::MmapThreadHandle::createMmapBuffer(int32_t minSizeFrames, struct audio_mmap_buffer_info *info) { - if (mThread == 0) { - return NO_INIT; - } return mThread->createMmapBuffer(minSizeFrames, info); } status_t AudioFlinger::MmapThreadHandle::getMmapPosition(struct audio_mmap_position *position) { - if (mThread == 0) { - return NO_INIT; - } return mThread->getMmapPosition(position); } @@ -7538,25 +7526,16 @@ status_t AudioFlinger::MmapThreadHandle::start(const AudioClient& client, audio_port_handle_t *handle) { - if (mThread == 0) { - return NO_INIT; - } return mThread->start(client, handle); } status_t AudioFlinger::MmapThreadHandle::stop(audio_port_handle_t handle) { - if (mThread == 0) { - return NO_INIT; - } return mThread->stop(handle); } status_t AudioFlinger::MmapThreadHandle::standby() { - if (mThread == 0) { - return NO_INIT; - } return mThread->standby(); } @@ -7588,7 +7567,7 @@ void AudioFlinger::MmapThread::disconnect() for (const sp &t : mActiveTracks) { stop(t->portId()); } - // this will cause the destruction of this thread. + // This will decrement references and may cause the destruction of this thread. if (isOutput()) { AudioSystem::releaseOutput(mId, streamType(), mSessionId); } else { -- 2.11.0