commit a5f44ebaf58911805b4fb7fb479b19fd89d2e39b
author Eric Laurent <elaurent@google.com> Mon Jun 25 11:38:29 2012 -0700
committer Eric Laurent <elaurent@google.com> Tue Jul 03 11:35:41 2012 -0700
tree 8221f3963c5be8d22bde7c3afdf98c1654333950
parent dd8104cc5367262f0e5f13df4e79f131e8d560bb

audioflinger: fix effect disconnect deadlock

Fix possible deadlock when several EffectHandles on the same
EffectModule are destroyed simultaneously:
 A wp on an EffectHandle should not be promoted to a local sp
with ThreadBase mutex held as the EffectHandle destructor can be
called when the sp gets out of scope which will call
ThreadBase::disconnectEffect() and try to acquire the mutex.

Use raw pointers instead of weak pointers for the list of handles
on an EffectModule.

Bug 6679606.

Change-Id: Ice8b602fb03a7d363c44ce3dced8a53540d96270

services/audioflinger/AudioFlinger.h

diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index b9ad7c7..cd157ec 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -516,7 +516,7 @@
                                         int *enabled,
                                         status_t *status);
                     void disconnectEffect(const sp< EffectModule>& effect,
-                                          const wp<EffectHandle>& handle,
+                                          EffectHandle *handle,
                                           bool unpinIfLast);
 
                     // return values for hasAudioSession (bit field)
@@ -1511,6 +1511,7 @@
             return mSessionId;
         }
         status_t    setEnabled(bool enabled);
+        status_t    setEnabled_l(bool enabled);
         bool isEnabled() const;
         bool isProcessEnabled() const;
 
@@ -1522,9 +1523,9 @@
         void        setThread(const wp<ThreadBase>& thread) { mThread = thread; }
         const wp<ThreadBase>& thread() { return mThread; }
 
-        status_t addHandle(const sp<EffectHandle>& handle);
-        void disconnect(const wp<EffectHandle>& handle, bool unpinIfLast);
-        size_t removeHandle (const wp<EffectHandle>& handle);
+        status_t addHandle(EffectHandle *handle);
+        size_t disconnect(EffectHandle *handle, bool unpinIfLast);
+        size_t removeHandle(EffectHandle *handle);
 
         effect_descriptor_t& desc() { return mDescriptor; }
         wp<EffectChain>&     chain() { return mChain; }
@@ -1537,10 +1538,13 @@
         void             setSuspended(bool suspended);
         bool             suspended() const;
 
-        sp<EffectHandle> controlHandle();
+        EffectHandle*    controlHandle_l();
 
         bool             isPinned() const { return mPinned; }
         void             unPin() { mPinned = false; }
+        bool             purgeHandles();
+        void             lock() { mLock.lock(); }
+        void             unlock() { mLock.unlock(); }
 
         status_t         dump(int fd, const Vector<String16>& args);
 
@@ -1567,7 +1571,7 @@
         effect_handle_t  mEffectInterface; // Effect module C API
         status_t            mStatus;    // initialization status
         effect_state        mState;     // current activation state
-        Vector< wp<EffectHandle> > mHandles;    // list of client handles
+        Vector<EffectHandle *> mHandles;    // list of client handles
                     // First handle in mHandles has highest priority and controls the effect module
         uint32_t mMaxDisableWaitCnt;    // maximum grace period before forcing an effect off after
                                         // sending disable command.
@@ -1625,6 +1629,8 @@
         int priority() const { return mPriority; }
         bool hasControl() const { return mHasControl; }
         sp<EffectModule> effect() const { return mEffect; }
+        // destroyed_l() must be called with the associated EffectModule mLock held
+        bool destroyed_l() const { return mDestroyed; }
 
         void dump(char* buffer, size_t size);
 
@@ -1643,6 +1649,8 @@
         bool mHasControl;                   // true if this handle is controlling the effect
         bool mEnabled;                      // cached enable state: needed when the effect is
                                             // restored after being suspended
+        bool mDestroyed;                    // Set to true by destructor. Access with EffectModule
+                                            // mLock held
     };
 
     // the EffectChain class represents a group of effects associated to one audio session.