commit 3de1bbfde9623ca4bf6d3a281d93857ae06e39a2
author Ytai Ben-Tsvi <ytai@google.com> Tue Jan 21 16:41:17 2020 -0800
committer Ytai Ben-Tsvi <ytai@google.com> Tue Jan 21 17:43:14 2020 -0800
tree 6ef063e1db869e5a7e54cf6bebe7392ac64ae3e8
parent 31f98910412ed7e023ffb240c49de63f958fd226

Fix race condition in Effects

Main problem was in AudioFlinger::EffectChain::EffectChain(), where
the thread* argument could possibly have been destroyed before the
creation of the weak pointer
(AudioFlinger::EffectChain::EffectCallback::mThread) around it.

While here, cleaned up a bunch of other pointer-related operations
in order to avoid trafficking in raw pointers to RefBase subclasses.

Bug: 147770363
Change-Id: I508ca94dd38a04e13f1a1c413f548001b961c721

services/audioflinger/Effects.h

diff --git a/services/audioflinger/Effects.h b/services/audioflinger/Effects.h
index 40bb226..1dfa1ae 100644
--- a/services/audioflinger/Effects.h
+++ b/services/audioflinger/Effects.h
@@ -402,7 +402,6 @@
 class EffectChain : public RefBase {
 public:
     EffectChain(const wp<ThreadBase>& wThread, audio_session_t sessionId);
-    EffectChain(ThreadBase *thread, audio_session_t sessionId);
     virtual ~EffectChain();
 
     // special key used for an entry in mSuspendedEffects keyed vector
@@ -513,8 +512,11 @@
 
     class EffectCallback :  public EffectCallbackInterface {
     public:
-        EffectCallback(EffectChain *chain, ThreadBase *thread, AudioFlinger *audioFlinger)
-            : mChain(chain), mThread(thread), mAudioFlinger(audioFlinger) {}
+        EffectCallback(const wp<EffectChain>& chain,
+                       const wp<ThreadBase>& thread)
+            : mChain(chain) {
+            setThread(thread);
+        }
 
         status_t createEffectHal(const effect_uuid_t *pEffectUuid,
                int32_t sessionId, int32_t deviceId, sp<EffectHalInterface> *effect) override;
@@ -550,7 +552,12 @@
         wp<EffectChain> chain() const override { return mChain; }
 
         wp<ThreadBase> thread() { return mThread; }
-        void setThread(ThreadBase *thread) { mThread = thread; };
+
+        void setThread(const wp<ThreadBase>& thread) {
+            mThread = thread;
+            sp<ThreadBase> p = thread.promote();
+            mAudioFlinger = p ? p->mAudioFlinger : nullptr;
+        }
 
     private:
         wp<EffectChain> mChain;
@@ -623,7 +630,8 @@
                 effect_descriptor_t *desc, int id)
             : EffectBase(callback, desc, id, AUDIO_SESSION_DEVICE, false),
                 mDevice(device), mManagerCallback(callback),
-                mMyCallback(new ProxyCallback(this, callback)) {}
+                mMyCallback(new ProxyCallback(wp<DeviceEffectProxy>(this),
+                                              callback)) {}
 
     status_t setEnabled(bool enabled, bool fromHandle) override;
     sp<DeviceEffectProxy> asDeviceEffectProxy() override { return this; }
@@ -649,7 +657,7 @@
 
     class ProxyCallback :  public EffectCallbackInterface {
     public:
-                ProxyCallback(DeviceEffectProxy *proxy,
+                ProxyCallback(const wp<DeviceEffectProxy>& proxy,
                         const sp<DeviceEffectManagerCallback>& callback)
                     : mProxy(proxy), mManagerCallback(callback) {}