Fix issue 3261656.

The problem can occur if a sample is started at the same time as the last AudioTrack callback
for a playing sample is called. At this time, allocateChannel() can be called concurrently with moveToFront()
which can cause an entry in mChannels being used by moveToFront() to be erased temporarily by allocateChannel().

The fix consists in making sure that the SoundPool mutex is held whenever play(), stop() or done() are called.

In addition, other potential weaknesses have been removed by making sure that the channel mutex is held while
starting, stopping and processing the AudioTrack call back.

To that purpose, a mechanism similar to the channel restart method is implemented to avoid stopping channels
from the AudioTrack call back but do it from the restart thread instead.

The sound effects SounPool management in AudioService has also been improved to make sure that the samples have
been loaded when a playback request is received and also to immediately release the SoundPool when the effects are
unloaded without waiting for the GC to occur.
The SoundPool.java class was modified to allow the use of a looper attached to the thread in which the sample
loaded listener is running and not to the thread in which the SoundPool is created.

The maximum number of samples that can be loaded in a SoundPool lifetime as been increased from 255 to 65535.

Change-Id: I368a3bdfda4239f807f857c3e97b70f6b31b0af3
diff --git a/media/java/android/media/AudioService.java b/media/java/android/media/AudioService.java
index 97fcf27..6a79384 100644
--- a/media/java/android/media/AudioService.java
+++ b/media/java/android/media/AudioService.java
@@ -275,6 +275,15 @@
     // in call audio)
     private static final int SCO_STATE_ACTIVE_EXTERNAL = 2;
 
+    // true if boot sequence has been completed
+    private boolean mBootCompleted;
+    // listener for SoundPool sample load completion indication
+    private SoundPoolCallback mSoundPoolCallBack;
+    // thread for SoundPool listener
+    private SoundPoolListenerThread mSoundPoolListenerThread;
+    // message looper for SoundPool listener
+    private Looper mSoundPoolLooper = null;
+
     ///////////////////////////////////////////////////////////////////////////
     // Construction
     ///////////////////////////////////////////////////////////////////////////
@@ -308,7 +317,6 @@
         setRingerModeInt(getRingerMode(), false);
 
         AudioSystem.setErrorCallback(mAudioSystemCallback);
-        loadSoundEffects();
 
         mBluetoothHeadsetDevice = null;
         BluetoothAdapter adapter = BluetoothAdapter.getDefaultAdapter();
@@ -868,12 +876,42 @@
      * This method must be called at when sound effects are enabled
      */
     public boolean loadSoundEffects() {
+        int status;
+
         synchronized (mSoundEffectsLock) {
+            if (!mBootCompleted) {
+                Log.w(TAG, "loadSoundEffects() called before boot complete");
+                return false;
+            }
+
             if (mSoundPool != null) {
                 return true;
             }
             mSoundPool = new SoundPool(NUM_SOUNDPOOL_CHANNELS, AudioSystem.STREAM_SYSTEM, 0);
             if (mSoundPool == null) {
+                Log.w(TAG, "loadSoundEffects() could not allocate sound pool");
+                return false;
+            }
+
+            try {
+                mSoundPoolCallBack = null;
+                mSoundPoolListenerThread = new SoundPoolListenerThread();
+                mSoundPoolListenerThread.start();
+                // Wait for mSoundPoolCallBack to be set by the other thread
+                mSoundEffectsLock.wait();
+            } catch (InterruptedException e) {
+                Log.w(TAG, "Interrupted while waiting sound pool listener thread.");
+            }
+
+            if (mSoundPoolCallBack == null) {
+                Log.w(TAG, "loadSoundEffects() could not create SoundPool listener or thread");
+                if (mSoundPoolLooper != null) {
+                    mSoundPoolLooper.quit();
+                    mSoundPoolLooper = null;
+                }
+                mSoundPoolListenerThread = null;
+                mSoundPool.release();
+                mSoundPool = null;
                 return false;
             }
             /*
@@ -891,26 +929,49 @@
              * If load succeeds, value in SOUND_EFFECT_FILES_MAP[effect][1] is > 0:
              * this indicates we have a valid sample loaded for this effect.
              */
+
             for (int effect = 0; effect < AudioManager.NUM_SOUND_EFFECTS; effect++) {
                 // Do not load sample if this effect uses the MediaPlayer
                 if (SOUND_EFFECT_FILES_MAP[effect][1] == 0) {
                     continue;
                 }
                 if (poolId[SOUND_EFFECT_FILES_MAP[effect][0]] == -1) {
-                    String filePath = Environment.getRootDirectory() + SOUND_EFFECTS_PATH + SOUND_EFFECT_FILES[SOUND_EFFECT_FILES_MAP[effect][0]];
+                    String filePath = Environment.getRootDirectory()
+                            + SOUND_EFFECTS_PATH
+                            + SOUND_EFFECT_FILES[SOUND_EFFECT_FILES_MAP[effect][0]];
                     int sampleId = mSoundPool.load(filePath, 0);
                     SOUND_EFFECT_FILES_MAP[effect][1] = sampleId;
                     poolId[SOUND_EFFECT_FILES_MAP[effect][0]] = sampleId;
                     if (sampleId <= 0) {
                         Log.w(TAG, "Soundpool could not load file: "+filePath);
                     }
+                    mSoundPoolCallBack.setLastSample(sampleId);
                 } else {
                     SOUND_EFFECT_FILES_MAP[effect][1] = poolId[SOUND_EFFECT_FILES_MAP[effect][0]];
                 }
             }
+            // wait for all samples to be loaded
+            try {
+                mSoundEffectsLock.wait();
+                status = mSoundPoolCallBack.status();
+            } catch (java.lang.InterruptedException e) {
+                status = -1;
+            }
+            if (mSoundPoolLooper != null) {
+                mSoundPoolLooper.quit();
+                mSoundPoolLooper = null;
+            }
+            mSoundPoolListenerThread = null;
+            if (status != 0) {
+                Log.w(TAG,
+                        "loadSoundEffects(), Error "
+                                + mSoundPoolCallBack.status()
+                                + " while loading samples");
+                mSoundPool.release();
+                mSoundPool = null;
+            }
         }
-
-        return true;
+        return (status == 0);
     }
 
     /**
@@ -923,6 +984,9 @@
             if (mSoundPool == null) {
                 return;
             }
+
+            mAudioHandler.removeMessages(MSG_PLAY_SOUND_EFFECT);
+
             int[] poolId = new int[SOUND_EFFECT_FILES.length];
             for (int fileIdx = 0; fileIdx < SOUND_EFFECT_FILES.length; fileIdx++) {
                 poolId[fileIdx] = 0;
@@ -938,10 +1002,60 @@
                     poolId[SOUND_EFFECT_FILES_MAP[effect][0]] = -1;
                 }
             }
+            mSoundPool.release();
             mSoundPool = null;
         }
     }
 
+    class SoundPoolListenerThread extends Thread {
+        public SoundPoolListenerThread() {
+            super("SoundPoolListenerThread");
+        }
+
+        @Override
+        public void run() {
+
+            Looper.prepare();
+            mSoundPoolLooper = Looper.myLooper();
+
+            synchronized (mSoundEffectsLock) {
+                if (mSoundPool != null) {
+                    mSoundPoolCallBack = new SoundPoolCallback();
+                    mSoundPool.setOnLoadCompleteListener(mSoundPoolCallBack);
+                }
+                mSoundEffectsLock.notify();
+            }
+            Looper.loop();
+        }
+    }
+
+    private final class SoundPoolCallback implements
+            android.media.SoundPool.OnLoadCompleteListener {
+
+        int mStatus;
+        int mLastSample;
+
+        public int status() {
+            return mStatus;
+        }
+
+        public void setLastSample(int sample) {
+            mLastSample = sample;
+        }
+
+        public void onLoadComplete(SoundPool soundPool, int sampleId, int status) {
+            synchronized (mSoundEffectsLock) {
+                if (status != 0) {
+                    mStatus = status;
+                }
+                if (sampleId == mLastSample) {
+                    Log.e(TAG, "onLoadComplete last sample loaded");
+                    mSoundEffectsLock.notify();
+                }
+            }
+        }
+    }
+
     /** @see AudioManager#reloadAudioSettings() */
     public void reloadAudioSettings() {
         // restore ringer mode, ringer mode affected streams, mute affected streams and vibrate settings
@@ -2130,6 +2244,8 @@
                     mContext.sendStickyBroadcast(newIntent);
                 }
             } else if (action.equals(Intent.ACTION_BOOT_COMPLETED)) {
+                mBootCompleted = true;
+                loadSoundEffects();
                 Intent newIntent = new Intent(AudioManager.ACTION_SCO_AUDIO_STATE_CHANGED);
                 newIntent.putExtra(AudioManager.EXTRA_SCO_AUDIO_STATE,
                         AudioManager.SCO_AUDIO_STATE_DISCONNECTED);
diff --git a/media/java/android/media/SoundPool.java b/media/java/android/media/SoundPool.java
index 18ce5c1..5e9c018 100644
--- a/media/java/android/media/SoundPool.java
+++ b/media/java/android/media/SoundPool.java
@@ -141,17 +141,6 @@
             throw new RuntimeException("Native setup failed");
         }
         mLock = new Object();
-
-        // setup message handler
-        Looper looper;
-        if ((looper = Looper.myLooper()) != null) {
-            mEventHandler = new EventHandler(this, looper);
-        } else if ((looper = Looper.getMainLooper()) != null) {
-            mEventHandler = new EventHandler(this, looper);
-        } else {
-            mEventHandler = null;
-        }
-
     }
 
     /**
@@ -427,6 +416,19 @@
     public void setOnLoadCompleteListener(OnLoadCompleteListener listener)
     {
         synchronized(mLock) {
+            if (listener != null) {
+                // setup message handler
+                Looper looper;
+                if ((looper = Looper.myLooper()) != null) {
+                    mEventHandler = new EventHandler(this, looper);
+                } else if ((looper = Looper.getMainLooper()) != null) {
+                    mEventHandler = new EventHandler(this, looper);
+                } else {
+                    mEventHandler = null;
+                }
+            } else {
+                mEventHandler = null;
+            }
             mOnLoadCompleteListener = listener;
         }
     }
diff --git a/media/jni/soundpool/SoundPool.cpp b/media/jni/soundpool/SoundPool.cpp
index fef880b..1e2d6ce 100644
--- a/media/jni/soundpool/SoundPool.cpp
+++ b/media/jni/soundpool/SoundPool.cpp
@@ -81,10 +81,10 @@
     quit();
 
     Mutex::Autolock lock(&mLock);
+
     mChannels.clear();
     if (mChannelPool)
         delete [] mChannelPool;
-
     // clean up samples
     LOGV("clear samples");
     mSamples.clear();
@@ -96,8 +96,19 @@
 void SoundPool::addToRestartList(SoundChannel* channel)
 {
     Mutex::Autolock lock(&mRestartLock);
-    mRestart.push_back(channel);
-    mCondition.signal();
+    if (!mQuit) {
+        mRestart.push_back(channel);
+        mCondition.signal();
+    }
+}
+
+void SoundPool::addToStopList(SoundChannel* channel)
+{
+    Mutex::Autolock lock(&mRestartLock);
+    if (!mQuit) {
+        mStop.push_back(channel);
+        mCondition.signal();
+    }
 }
 
 int SoundPool::beginThread(void* arg)
@@ -114,17 +125,38 @@
         LOGV("awake");
         if (mQuit) break;
 
+        while (!mStop.empty()) {
+            SoundChannel* channel;
+            LOGV("Getting channel from stop list");
+            List<SoundChannel* >::iterator iter = mStop.begin();
+            channel = *iter;
+            mStop.erase(iter);
+            mRestartLock.unlock();
+            if (channel != 0) {
+                Mutex::Autolock lock(&mLock);
+                channel->stop();
+            }
+            mRestartLock.lock();
+            if (mQuit) break;
+        }
+
         while (!mRestart.empty()) {
             SoundChannel* channel;
             LOGV("Getting channel from list");
             List<SoundChannel*>::iterator iter = mRestart.begin();
             channel = *iter;
             mRestart.erase(iter);
-            if (channel) channel->nextEvent();
+            mRestartLock.unlock();
+            if (channel != 0) {
+                Mutex::Autolock lock(&mLock);
+                channel->nextEvent();
+            }
+            mRestartLock.lock();
             if (mQuit) break;
         }
     }
 
+    mStop.clear();
     mRestart.clear();
     mCondition.signal();
     mRestartLock.unlock();
@@ -208,43 +240,43 @@
 int SoundPool::play(int sampleID, float leftVolume, float rightVolume,
         int priority, int loop, float rate)
 {
-    LOGV("sampleID=%d, leftVolume=%f, rightVolume=%f, priority=%d, loop=%d, rate=%f",
+    LOGV("play sampleID=%d, leftVolume=%f, rightVolume=%f, priority=%d, loop=%d, rate=%f",
             sampleID, leftVolume, rightVolume, priority, loop, rate);
     sp<Sample> sample;
     SoundChannel* channel;
     int channelID;
 
-    // scope for lock
-    {
-        Mutex::Autolock lock(&mLock);
+    Mutex::Autolock lock(&mLock);
 
-        // is sample ready?
-        sample = findSample(sampleID);
-        if ((sample == 0) || (sample->state() != Sample::READY)) {
-            LOGW("  sample %d not READY", sampleID);
-            return 0;
-        }
-
-        dump();
-
-        // allocate a channel
-        channel = allocateChannel(priority);
-
-        // no channel allocated - return 0
-        if (!channel) {
-            LOGV("No channel allocated");
-            return 0;
-        }
-
-        channelID = ++mNextChannelID;
+    if (mQuit) {
+        return 0;
+    }
+    // is sample ready?
+    sample = findSample(sampleID);
+    if ((sample == 0) || (sample->state() != Sample::READY)) {
+        LOGW("  sample %d not READY", sampleID);
+        return 0;
     }
 
-    LOGV("channel state = %d", channel->state());
+    dump();
+
+    // allocate a channel
+    channel = allocateChannel_l(priority);
+
+    // no channel allocated - return 0
+    if (!channel) {
+        LOGV("No channel allocated");
+        return 0;
+    }
+
+    channelID = ++mNextChannelID;
+
+    LOGV("play channel %p state = %d", channel, channel->state());
     channel->play(sample, channelID, leftVolume, rightVolume, priority, loop, rate);
     return channelID;
 }
 
-SoundChannel* SoundPool::allocateChannel(int priority)
+SoundChannel* SoundPool::allocateChannel_l(int priority)
 {
     List<SoundChannel*>::iterator iter;
     SoundChannel* channel = NULL;
@@ -273,7 +305,7 @@
 }
 
 // move a channel from its current position to the front of the list
-void SoundPool::moveToFront(SoundChannel* channel)
+void SoundPool::moveToFront_l(SoundChannel* channel)
 {
     for (List<SoundChannel*>::iterator iter = mChannels.begin(); iter != mChannels.end(); ++iter) {
         if (*iter == channel) {
@@ -378,10 +410,9 @@
 }
 
 // call with lock held
-void SoundPool::done(SoundChannel* channel)
+void SoundPool::done_l(SoundChannel* channel)
 {
-    LOGV("done(%d)", channel->channelID());
-
+    LOGV("done_l(%d)", channel->channelID());
     // if "stolen", play next event
     if (channel->nextChannelID() != 0) {
         LOGV("add to restart list");
@@ -391,7 +422,7 @@
     // return to idle state
     else {
         LOGV("move to front");
-        moveToFront(channel);
+        moveToFront_l(channel);
     }
 }
 
@@ -511,81 +542,88 @@
     mSoundPool = soundPool;
 }
 
+// call with sound pool lock held
 void SoundChannel::play(const sp<Sample>& sample, int nextChannelID, float leftVolume,
         float rightVolume, int priority, int loop, float rate)
 {
     AudioTrack* oldTrack;
+    AudioTrack* newTrack;
+    status_t status;
 
-    LOGV("play %p: sampleID=%d, channelID=%d, leftVolume=%f, rightVolume=%f, priority=%d, loop=%d, rate=%f",
-            this, sample->sampleID(), nextChannelID, leftVolume, rightVolume, priority, loop, rate);
+    { // scope for the lock
+        Mutex::Autolock lock(&mLock);
 
-    // if not idle, this voice is being stolen
-    if (mState != IDLE) {
-        LOGV("channel %d stolen - event queued for channel %d", channelID(), nextChannelID);
-        mNextEvent.set(sample, nextChannelID, leftVolume, rightVolume, priority, loop, rate);
-        stop();
-        return;
-    }
+        LOGV("SoundChannel::play %p: sampleID=%d, channelID=%d, leftVolume=%f, rightVolume=%f,"
+                " priority=%d, loop=%d, rate=%f",
+                this, sample->sampleID(), nextChannelID, leftVolume, rightVolume,
+                priority, loop, rate);
 
-    // initialize track
-    int afFrameCount;
-    int afSampleRate;
-    int streamType = mSoundPool->streamType();
-    if (AudioSystem::getOutputFrameCount(&afFrameCount, streamType) != NO_ERROR) {
-        afFrameCount = kDefaultFrameCount;
-    }
-    if (AudioSystem::getOutputSamplingRate(&afSampleRate, streamType) != NO_ERROR) {
-        afSampleRate = kDefaultSampleRate;
-    }
-    int numChannels = sample->numChannels();
-    uint32_t sampleRate = uint32_t(float(sample->sampleRate()) * rate + 0.5);
-    uint32_t totalFrames = (kDefaultBufferCount * afFrameCount * sampleRate) / afSampleRate;
-    uint32_t bufferFrames = (totalFrames + (kDefaultBufferCount - 1)) / kDefaultBufferCount;
-    uint32_t frameCount = 0;
+        // if not idle, this voice is being stolen
+        if (mState != IDLE) {
+            LOGV("channel %d stolen - event queued for channel %d", channelID(), nextChannelID);
+            mNextEvent.set(sample, nextChannelID, leftVolume, rightVolume, priority, loop, rate);
+            stop_l();
+            return;
+        }
 
-    if (loop) {
-        frameCount = sample->size()/numChannels/((sample->format() == AudioSystem::PCM_16_BIT) ? sizeof(int16_t) : sizeof(uint8_t));
-    }
+        // initialize track
+        int afFrameCount;
+        int afSampleRate;
+        int streamType = mSoundPool->streamType();
+        if (AudioSystem::getOutputFrameCount(&afFrameCount, streamType) != NO_ERROR) {
+            afFrameCount = kDefaultFrameCount;
+        }
+        if (AudioSystem::getOutputSamplingRate(&afSampleRate, streamType) != NO_ERROR) {
+            afSampleRate = kDefaultSampleRate;
+        }
+        int numChannels = sample->numChannels();
+        uint32_t sampleRate = uint32_t(float(sample->sampleRate()) * rate + 0.5);
+        uint32_t totalFrames = (kDefaultBufferCount * afFrameCount * sampleRate) / afSampleRate;
+        uint32_t bufferFrames = (totalFrames + (kDefaultBufferCount - 1)) / kDefaultBufferCount;
+        uint32_t frameCount = 0;
+
+        if (loop) {
+            frameCount = sample->size()/numChannels/
+                ((sample->format() == AudioSystem::PCM_16_BIT) ? sizeof(int16_t) : sizeof(uint8_t));
+        }
 
 #ifndef USE_SHARED_MEM_BUFFER
-    // Ensure minimum audio buffer size in case of short looped sample
-    if(frameCount < totalFrames) {
-        frameCount = totalFrames;
-    }
+        // Ensure minimum audio buffer size in case of short looped sample
+        if(frameCount < totalFrames) {
+            frameCount = totalFrames;
+        }
 #endif
 
-    AudioTrack* newTrack;
+        // mToggle toggles each time a track is started on a given channel.
+        // The toggle is concatenated with the SoundChannel address and passed to AudioTrack
+        // as callback user data. This enables the detection of callbacks received from the old
+        // audio track while the new one is being started and avoids processing them with
+        // wrong audio audio buffer size  (mAudioBufferSize)
+        unsigned long toggle = mToggle ^ 1;
+        void *userData = (void *)((unsigned long)this | toggle);
+        uint32_t channels = (numChannels == 2) ?
+                AudioSystem::CHANNEL_OUT_STEREO : AudioSystem::CHANNEL_OUT_MONO;
 
-    // mToggle toggles each time a track is started on a given channel.
-    // The toggle is concatenated with the SoundChannel address and passed to AudioTrack
-    // as callback user data. This enables the detection of callbacks received from the old
-    // audio track while the new one is being started and avoids processing them with 
-    // wrong audio audio buffer size  (mAudioBufferSize)
-    unsigned long toggle = mToggle ^ 1;
-    void *userData = (void *)((unsigned long)this | toggle);
-    uint32_t channels = (numChannels == 2) ? AudioSystem::CHANNEL_OUT_STEREO : AudioSystem::CHANNEL_OUT_MONO;
-
+        // do not create a new audio track if current track is compatible with sample parameters
 #ifdef USE_SHARED_MEM_BUFFER
-    newTrack = new AudioTrack(streamType, sampleRate, sample->format(),
-            channels, sample->getIMemory(), 0, callback, userData);
+        newTrack = new AudioTrack(streamType, sampleRate, sample->format(),
+                channels, sample->getIMemory(), 0, callback, userData);
 #else
-    newTrack = new AudioTrack(streamType, sampleRate, sample->format(),
-            channels, frameCount, 0, callback, userData, bufferFrames);
+        newTrack = new AudioTrack(streamType, sampleRate, sample->format(),
+                channels, frameCount, 0, callback, userData, bufferFrames);
 #endif
-    if (newTrack->initCheck() != NO_ERROR) {
-        LOGE("Error creating AudioTrack");
-        delete newTrack;
-        return;
-    }
-    LOGV("setVolume %p", newTrack);
-    newTrack->setVolume(leftVolume, rightVolume);
-    newTrack->setLoop(0, frameCount, loop);
+        oldTrack = mAudioTrack;
+        status = newTrack->initCheck();
+        if (status != NO_ERROR) {
+            LOGE("Error creating AudioTrack");
+            goto exit;
+        }
+        LOGV("setVolume %p", newTrack);
+        newTrack->setVolume(leftVolume, rightVolume);
+        newTrack->setLoop(0, frameCount, loop);
 
-    {
-        Mutex::Autolock lock(&mLock);
         // From now on, AudioTrack callbacks recevieved with previous toggle value will be ignored.
         mToggle = toggle;
-        oldTrack = mAudioTrack;
         mAudioTrack = newTrack;
         mPos = 0;
         mSample = sample;
@@ -602,8 +640,13 @@
         mAudioBufferSize = newTrack->frameCount()*newTrack->frameSize();
     }
 
+exit:
     LOGV("delete oldTrack %p", oldTrack);
     delete oldTrack;
+    if (status != NO_ERROR) {
+        delete newTrack;
+        mAudioTrack = NULL;
+    }
 }
 
 void SoundChannel::nextEvent()
@@ -639,29 +682,44 @@
 
 void SoundChannel::callback(int event, void* user, void *info)
 {
-    unsigned long toggle = (unsigned long)user & 1;
     SoundChannel* channel = static_cast<SoundChannel*>((void *)((unsigned long)user & ~1));
     
-    if (channel->mToggle != toggle) {
-        LOGV("callback with wrong toggle");
-        return;
-    }
-    channel->process(event, info);
+    channel->process(event, info, (unsigned long)user & 1);
 }
 
-void SoundChannel::process(int event, void *info)
+void SoundChannel::process(int event, void *info, unsigned long toggle)
 {
     //LOGV("process(%d)", mChannelID);
+
+    Mutex::Autolock lock(&mLock);
+
+    AudioTrack::Buffer* b = NULL;
+    if (event == AudioTrack::EVENT_MORE_DATA) {
+       b = static_cast<AudioTrack::Buffer *>(info);
+    }
+
+    if (mToggle != toggle) {
+        LOGV("process wrong toggle %p channel %d", this, mChannelID);
+        if (b != NULL) {
+            b->size = 0;
+        }
+        return;
+    }
+
     sp<Sample> sample = mSample;
 
 //    LOGV("SoundChannel::process event %d", event);
 
     if (event == AudioTrack::EVENT_MORE_DATA) {
-       AudioTrack::Buffer* b = static_cast<AudioTrack::Buffer *>(info);
 
         // check for stop state
         if (b->size == 0) return;
 
+        if (mState == IDLE) {
+            b->size = 0;
+            return;
+        }
+
         if (sample != 0) {
             // fill buffer
             uint8_t* q = (uint8_t*) b->i8;
@@ -674,14 +732,14 @@
                     count = b->size;
                 }
                 memcpy(q, p, count);
-                LOGV("fill: q=%p, p=%p, mPos=%u, b->size=%u, count=%d", q, p, mPos, b->size, count);
+//              LOGV("fill: q=%p, p=%p, mPos=%u, b->size=%u, count=%d", q, p, mPos, b->size, count);
             } else if (mPos < mAudioBufferSize) {
                 count = mAudioBufferSize - mPos;
                 if (count > b->size) {
                     count = b->size;
                 }
                 memset(q, 0, count);
-                LOGV("fill extra: q=%p, mPos=%u, b->size=%u, count=%d", q, mPos, b->size, count);
+//              LOGV("fill extra: q=%p, mPos=%u, b->size=%u, count=%d", q, mPos, b->size, count);
             }
 
             mPos += count;
@@ -689,16 +747,16 @@
             //LOGV("buffer=%p, [0]=%d", b->i16, b->i16[0]);
         }
     } else if (event == AudioTrack::EVENT_UNDERRUN) {
-        LOGV("stopping track");
-        stop();
+        LOGV("process %p channel %d EVENT_UNDERRUN", this, mChannelID);
+        mSoundPool->addToStopList(this);
     } else if (event == AudioTrack::EVENT_LOOP_END) {
-        LOGV("End loop: %d", *(int *)info);
+        LOGV("End loop %p channel %d count %d", this, mChannelID, *(int *)info);
     }
 }
 
 
 // call with lock held
-void SoundChannel::stop_l()
+bool SoundChannel::doStop_l()
 {
     if (mState != IDLE) {
         setVolume_l(0, 0);
@@ -707,16 +765,31 @@
         mSample.clear();
         mState = IDLE;
         mPriority = IDLE_PRIORITY;
+        return true;
+    }
+    return false;
+}
+
+// call with lock held and sound pool lock held
+void SoundChannel::stop_l()
+{
+    if (doStop_l()) {
+        mSoundPool->done_l(this);
     }
 }
 
+// call with sound pool lock held
 void SoundChannel::stop()
 {
+    bool stopped;
     {
         Mutex::Autolock lock(&mLock);
-        stop_l();
+        stopped = doStop_l();
     }
-    mSoundPool->done(this);
+
+    if (stopped) {
+        mSoundPool->done_l(this);
+    }
 }
 
 //FIXME: Pause is a little broken right now
@@ -791,21 +864,24 @@
 {
     Mutex::Autolock lock(&mLock);
     if (mAudioTrack != 0 && mSample.get() != 0) {
-        mAudioTrack->setLoop(0, mSample->size()/mNumChannels/((mSample->format() == AudioSystem::PCM_16_BIT) ? sizeof(int16_t) : sizeof(uint8_t)), loop);
+        uint32_t loopEnd = mSample->size()/mNumChannels/
+            ((mSample->format() == AudioSystem::PCM_16_BIT) ? sizeof(int16_t) : sizeof(uint8_t));
+        mAudioTrack->setLoop(0, loopEnd, loop);
         mLoop = loop;
     }
 }
 
 SoundChannel::~SoundChannel()
 {
-    LOGV("SoundChannel destructor");
-    if (mAudioTrack) {
-        LOGV("stop track");
-        mAudioTrack->stop();
-        delete mAudioTrack;
+    LOGV("SoundChannel destructor %p", this);
+    {
+        Mutex::Autolock lock(&mLock);
+        clearNextEvent();
+        doStop_l();
     }
-    clearNextEvent();
-    mSample.clear();
+    // do not call AudioTrack destructor with mLock held as it will wait for the AudioTrack
+    // callback thread to exit which may need to execute process() and acquire the mLock.
+    delete mAudioTrack;
 }
 
 void SoundChannel::dump()
@@ -817,7 +893,7 @@
 void SoundEvent::set(const sp<Sample>& sample, int channelID, float leftVolume,
             float rightVolume, int priority, int loop, float rate)
 {
-    mSample =sample;
+    mSample = sample;
     mChannelID = channelID;
     mLeftVolume = leftVolume;
     mRightVolume = rightVolume;
diff --git a/media/jni/soundpool/SoundPool.h b/media/jni/soundpool/SoundPool.h
index 1b0fd38..6010aac 100644
--- a/media/jni/soundpool/SoundPool.h
+++ b/media/jni/soundpool/SoundPool.h
@@ -142,7 +142,8 @@
 
 private:
     static void callback(int event, void* user, void *info);
-    void process(int event, void *info);
+    void process(int event, void *info, unsigned long toggle);
+    bool doStop_l();
 
     SoundPool*          mSoundPool;
     AudioTrack*         mAudioTrack;
@@ -184,7 +185,7 @@
     void sampleLoaded(int sampleID);
 
     // called from AudioTrack thread
-    void done(SoundChannel* channel);
+    void done_l(SoundChannel* channel);
 
     // callback function
     void setCallback(SoundPoolCallback* callback, void* user);
@@ -197,13 +198,14 @@
     sp<Sample> findSample(int sampleID) { return mSamples.valueFor(sampleID); }
     SoundChannel* findChannel (int channelID);
     SoundChannel* findNextChannel (int channelID);
-    SoundChannel* allocateChannel(int priority);
-    void moveToFront(SoundChannel* channel);
+    SoundChannel* allocateChannel_l(int priority);
+    void moveToFront_l(SoundChannel* channel);
     void notify(SoundPoolEvent event);
     void dump();
 
     // restart thread
     void addToRestartList(SoundChannel* channel);
+    void addToStopList(SoundChannel* channel);
     static int beginThread(void* arg);
     int run();
     void quit();
@@ -215,6 +217,7 @@
     SoundChannel*           mChannelPool;
     List<SoundChannel*>     mChannels;
     List<SoundChannel*>     mRestart;
+    List<SoundChannel*>     mStop;
     DefaultKeyedVector< int, sp<Sample> >   mSamples;
     int                     mMaxChannels;
     int                     mStreamType;
diff --git a/media/jni/soundpool/SoundPoolThread.h b/media/jni/soundpool/SoundPoolThread.h
index bbd35e0..d388388 100644
--- a/media/jni/soundpool/SoundPoolThread.h
+++ b/media/jni/soundpool/SoundPoolThread.h
@@ -31,10 +31,8 @@
     SoundPoolMsg() : mMessageType(INVALID), mData(0) {}
     SoundPoolMsg(MessageType MessageType, int data) :
         mMessageType(MessageType), mData(data) {}
-    uint8_t         mMessageType;
-    uint8_t         mData;
-    uint8_t         mData2;
-    uint8_t         mData3;
+    uint16_t         mMessageType;
+    uint16_t         mData;
 };
 
 /*