commit 69d41cc6ca5d63e5484874dc3dcaf78530b55b6a
author Mikhail Naganov <mnaganov@google.com> Fri Jul 31 17:36:08 2020 -0700
committer Mikhail Naganov <mnaganov@google.com> Mon Aug 03 23:44:56 2020 +0000
tree 423c2ef2a2dc0410c94c32ef5736fcfd5574d777
parent 1fc724e954b4b0384ae12b095424214754c15d94

Avoid storing 'this' in a wp<AudioEffect> while in constructor

Storing 'this' into wp<AudioEffect> while running the constructor
of an AudioEffect can lead to premature instance destruction
because the owner of this weak pointer may promote to sp<>
on a thread running in parallel to the constructor.

Fixed by removing a call to AudioEffect::set from the constructor
and moving the responsibility for calling it to client code.

Bug: 162012829
Bug: 162323621
Test: repro steps from the bug
      atest android.media.cts.VisualizerTest
Change-Id: Ice5f510b1c9020b5ba73bd6223c1669ab8eae24c
Merged-In: Ice5f510b1c9020b5ba73bd6223c1669ab8eae24c

services/audiopolicy/service/AudioPolicyEffects.cpp

diff --git a/services/audiopolicy/service/AudioPolicyEffects.cpp b/services/audiopolicy/service/AudioPolicyEffects.cpp
index 738a279..d5cb395 100644
--- a/services/audiopolicy/service/AudioPolicyEffects.cpp
+++ b/services/audiopolicy/service/AudioPolicyEffects.cpp
@@ -121,8 +121,8 @@
         Vector <EffectDesc *> effects = mInputSources.valueAt(index)->mEffects;
         for (size_t i = 0; i < effects.size(); i++) {
             EffectDesc *effect = effects[i];
-            sp<AudioEffect> fx = new AudioEffect(NULL, String16("android"), &effect->mUuid, -1, 0,
-                                                 0, audioSession, input);
+            sp<AudioEffect> fx = new AudioEffect(String16("android"));
+            fx->set(NULL, &effect->mUuid, -1, 0, 0, audioSession, input);
             status_t status = fx->initCheck();
             if (status != NO_ERROR && status != ALREADY_EXISTS) {
                 ALOGW("addInputEffects(): failed to create Fx %s on source %d",
@@ -270,8 +270,8 @@
         Vector <EffectDesc *> effects = mOutputStreams.valueAt(index)->mEffects;
         for (size_t i = 0; i < effects.size(); i++) {
             EffectDesc *effect = effects[i];
-            sp<AudioEffect> fx = new AudioEffect(NULL, String16("android"), &effect->mUuid, 0, 0, 0,
-                                                 audioSession, output);
+            sp<AudioEffect> fx = new AudioEffect(String16("android"));
+            fx->set(NULL, &effect->mUuid, 0, 0, 0, audioSession, output);
             status_t status = fx->initCheck();
             if (status != NO_ERROR && status != ALREADY_EXISTS) {
                 ALOGE("addOutputSessionEffects(): failed to create Fx  %s on session %d",
@@ -967,11 +967,11 @@
     for (const auto& deviceEffectsIter : mDeviceEffects) {
         const auto& deviceEffects =  deviceEffectsIter.second;
         for (const auto& effectDesc : deviceEffects->mEffectDescriptors->mEffects) {
-            auto fx = std::make_unique<AudioEffect>(
-                        EFFECT_UUID_NULL, String16("android"), &effectDesc->mUuid, 0, nullptr,
-                        nullptr, AUDIO_SESSION_DEVICE, AUDIO_IO_HANDLE_NONE,
-                        AudioDeviceTypeAddr{deviceEffects->getDeviceType(),
-                                            deviceEffects->getDeviceAddress()});
+            auto fx = std::make_unique<AudioEffect>(String16("android"));
+            fx->set(EFFECT_UUID_NULL, &effectDesc->mUuid, 0, nullptr,
+                    nullptr, AUDIO_SESSION_DEVICE, AUDIO_IO_HANDLE_NONE,
+                    AudioDeviceTypeAddr{deviceEffects->getDeviceType(),
+                                        deviceEffects->getDeviceAddress()});
             status_t status = fx->initCheck();
             if (status != NO_ERROR && status != ALREADY_EXISTS) {
                 ALOGE("%s(): failed to create Fx %s on port type=%d address=%s", __func__,