From 48412dc92c4cc92a3f4821616e12290f5eeabd07 Mon Sep 17 00:00:00 2001
From: Jesper Tragardh <jesper.tragardh@sonymobile.com>
Date: Mon, 24 Mar 2014 14:12:43 +0100
Subject: Fix memory leak when filtering commands in insertCommand_l()

When AudioPolicyService::AudioCommandThread::insertCommand_l filters
a command from the queue, the AudioCommand* is removed from the
vector mAudioCommands by calling mAudioCommands.removeAt(). This will
destroy the pointer and compact the vector, but not delete the
AudioCommand object.

This patch adds code to delete a filtered out AudioCommand and its
data object mParam. However, mParam was void* and deleting a void
pointer is undefined behavior according to the C++
standard. Therefore, the data classes are modified to inherit
from an abstract base class with a virtual destructor.

The deletion of non-filtered commands in
AudioPolicyService::AudioCommandThread::threadLoop() was already
correct, as each case casts mParam to the proper type before
deleting.

This also fixes AOSP issue 36370.

Change-Id: I2b63838aff3608132aa496f964a929f78c4b6267
---
 services/audioflinger/AudioPolicyService.cpp | 14 +++++++++++---
 services/audioflinger/AudioPolicyService.h   | 23 ++++++++++++++++-------
 2 files changed, 27 insertions(+), 10 deletions(-)

(limited to 'services')

diff --git a/services/audioflinger/AudioPolicyService.cpp b/services/audioflinger/AudioPolicyService.cpp
index a37272d..6ea5324 100644
--- a/services/audioflinger/AudioPolicyService.cpp
+++ b/services/audioflinger/AudioPolicyService.cpp
@@ -678,6 +678,10 @@ AudioPolicyService::AudioCommandThread::~AudioCommandThread()
     if (!mAudioCommands.isEmpty()) {
         release_wake_lock(mName.string());
     }
+    for (size_t k=0; k < mAudioCommands.size(); k++) {
+        delete mAudioCommands[k]->mParam;
+        delete mAudioCommands[k];
+    }
     mAudioCommands.clear();
     delete mpToneGenerator;
 }
@@ -849,7 +853,7 @@ void AudioPolicyService::AudioCommandThread::startToneCommand(ToneGenerator::ton
     ToneData *data = new ToneData();
     data->mType = type;
     data->mStream = stream;
-    command->mParam = (void *)data;
+    command->mParam = data;
     Mutex::Autolock _l(mLock);
     insertCommand_l(command);
     ALOGV("AudioCommandThread() adding tone start type %d, stream %d", type, stream);
@@ -950,7 +954,7 @@ void AudioPolicyService::AudioCommandThread::stopOutputCommand(audio_io_handle_t
     data->mIO = output;
     data->mStream = stream;
     data->mSession = session;
-    command->mParam = (void *)data;
+    command->mParam = data;
     Mutex::Autolock _l(mLock);
     insertCommand_l(command);
     ALOGV("AudioCommandThread() adding stop output %d", output);
@@ -963,7 +967,7 @@ void AudioPolicyService::AudioCommandThread::releaseOutputCommand(audio_io_handl
     command->mCommand = RELEASE_OUTPUT;
     ReleaseOutputData *data = new ReleaseOutputData();
     data->mIO = output;
-    command->mParam = (void *)data;
+    command->mParam = data;
     Mutex::Autolock _l(mLock);
     insertCommand_l(command);
     ALOGV("AudioCommandThread() adding release output %d", output);
@@ -1052,6 +1056,10 @@ void AudioPolicyService::AudioCommandThread::insertCommand_l(AudioCommand *comma
         for (size_t k = i + 1; k < mAudioCommands.size(); k++) {
             if (mAudioCommands[k] == removedCommands[j]) {
                 ALOGV("suppressing command: %d", mAudioCommands[k]->mCommand);
+                // for commands that are not filtered,
+                // command->mParam is deleted in threadLoop
+                delete mAudioCommands[k]->mParam;
+                delete mAudioCommands[k];
                 mAudioCommands.removeAt(k);
                 break;
             }
diff --git a/services/audioflinger/AudioPolicyService.h b/services/audioflinger/AudioPolicyService.h
index ae053a9..a38160f 100644
--- a/services/audioflinger/AudioPolicyService.h
+++ b/services/audioflinger/AudioPolicyService.h
@@ -197,6 +197,8 @@ private:
                     void        insertCommand_l(AudioCommand *command, int delayMs = 0);
 
     private:
+        class AudioCommandData;
+
         // descriptor for requested tone playback event
         class AudioCommand {
 
@@ -211,41 +213,48 @@ private:
             Condition mCond; // condition for status return
             status_t mStatus; // command status
             bool mWaitStatus; // true if caller is waiting for status
-            void *mParam;     // command parameter (ToneData, VolumeData, ParametersData)
+            AudioCommandData *mParam;     // command specific parameter data
+        };
+
+        class AudioCommandData {
+        public:
+            virtual ~AudioCommandData() {}
+        protected:
+            AudioCommandData() {}
         };
 
-        class ToneData {
+        class ToneData : public AudioCommandData {
         public:
             ToneGenerator::tone_type mType; // tone type (START_TONE only)
             audio_stream_type_t mStream;    // stream type (START_TONE only)
         };
 
-        class VolumeData {
+        class VolumeData : public AudioCommandData {
         public:
             audio_stream_type_t mStream;
             float mVolume;
             audio_io_handle_t mIO;
         };
 
-        class ParametersData {
+        class ParametersData : public AudioCommandData {
         public:
             audio_io_handle_t mIO;
             String8 mKeyValuePairs;
         };
 
-        class VoiceVolumeData {
+        class VoiceVolumeData : public AudioCommandData {
         public:
             float mVolume;
         };
 
-        class StopOutputData {
+        class StopOutputData : public AudioCommandData {
         public:
             audio_io_handle_t mIO;
             audio_stream_type_t mStream;
             int mSession;
         };
 
-        class ReleaseOutputData {
+        class ReleaseOutputData : public AudioCommandData {
         public:
             audio_io_handle_t mIO;
         };
-- 
cgit v1.1