AudioPolicyManager: abort when client is removed while active

This can result in a refcount mismatch. It is safer to
abort immediately.

Test: Audio sanity, phone call
Bug: 112067674
Change-Id: I7835e75342b4c1aa3d6d910ee782d3bc818bd553
diff --git a/services/audiopolicy/common/managerdefinitions/include/AudioInputDescriptor.h b/services/audiopolicy/common/managerdefinitions/include/AudioInputDescriptor.h
index 72d5a8c..df3b41e 100644
--- a/services/audiopolicy/common/managerdefinitions/include/AudioInputDescriptor.h
+++ b/services/audiopolicy/common/managerdefinitions/include/AudioInputDescriptor.h
@@ -33,6 +33,7 @@
 // descriptor for audio inputs. Used to maintain current configuration of each opened audio input
 // and keep track of the usage of this input.
 class AudioInputDescriptor: public AudioPortConfig, public AudioIODescriptorInterface
+    , public ClientMapHandler<RecordClientDescriptor>
 {
 public:
     explicit AudioInputDescriptor(const sp<IOProfile>& profile,
@@ -42,10 +43,10 @@
 
     status_t    dump(int fd);
 
-    audio_io_handle_t             mIoHandle;       // input handle
-    audio_devices_t               mDevice;         // current device this input is routed to
-    AudioMix                      *mPolicyMix;     // non NULL when used by a dynamic policy
-    const sp<IOProfile>           mProfile;        // I/O profile this output derives from
+    audio_io_handle_t   mIoHandle = AUDIO_IO_HANDLE_NONE; // input handle
+    audio_devices_t     mDevice = AUDIO_DEVICE_NONE;  // current device this input is routed to
+    AudioMix            *mPolicyMix = nullptr;        // non NULL when used by a dynamic policy
+    const sp<IOProfile> mProfile;                     // I/O profile this output derives from
 
     virtual void toAudioPortConfig(struct audio_port_config *dstConfig,
             const struct audio_port_config *srcConfig = NULL) const;
@@ -82,7 +83,6 @@
     void stop();
     void close();
 
-    RecordClientMap& clientsMap() { return mClients; }
     RecordClientVector getClientsForSession(audio_session_t session);
     RecordClientVector clientsList(bool activeOnly = false,
         audio_source_t source = AUDIO_SOURCE_DEFAULT, bool preferredDeviceOnly = false) const;
@@ -91,8 +91,8 @@
 
     void updateClientRecordingConfiguration(int event, const sp<RecordClientDescriptor>& client);
 
-    audio_patch_handle_t          mPatchHandle;
-    audio_port_handle_t           mId;
+    audio_patch_handle_t mPatchHandle = AUDIO_PATCH_HANDLE_NONE;
+    audio_port_handle_t  mId = AUDIO_PORT_HANDLE_NONE;
     // Because a preemptible capture session can preempt another one, we end up in an endless loop
     // situation were each session is allowed to restart after being preempted,
     // thus preempting the other one which restarts and so on.
@@ -100,10 +100,8 @@
     // a particular input started and prevent preemption of this active input by this session.
     // We also inherit sessions from the preempted input to avoid a 3 way preemption loop etc...
     SortedVector<audio_session_t> mPreemptedSessions;
-    AudioPolicyClientInterface *mClientInterface;
-    int32_t mGlobalActiveCount;  // non-client-specific activity ref count
-
-    RecordClientMap mClients;
+    AudioPolicyClientInterface * const mClientInterface;
+    int32_t mGlobalActiveCount = 0;  // non-client-specific activity ref count
 };
 
 class AudioInputCollection :
diff --git a/services/audiopolicy/common/managerdefinitions/include/AudioOutputDescriptor.h b/services/audiopolicy/common/managerdefinitions/include/AudioOutputDescriptor.h
index cb4206a..257209a 100644
--- a/services/audiopolicy/common/managerdefinitions/include/AudioOutputDescriptor.h
+++ b/services/audiopolicy/common/managerdefinitions/include/AudioOutputDescriptor.h
@@ -18,7 +18,6 @@
 
 #include <sys/types.h>
 
-#include <audio_utils/SimpleLog.h>
 #include <utils/Errors.h>
 #include <utils/Timers.h>
 #include <utils/KeyedVector.h>
@@ -38,6 +37,7 @@
 // descriptor for audio outputs. Used to maintain current configuration of each opened audio output
 // and keep track of the usage of this output by each audio stream type.
 class AudioOutputDescriptor: public AudioPortConfig, public AudioIODescriptorInterface
+    , public ClientMapHandler<TrackClientDescriptor>
 {
 public:
     AudioOutputDescriptor(const sp<AudioPort>& port,
@@ -61,9 +61,21 @@
                            audio_devices_t device,
                            uint32_t delayMs,
                            bool force);
+
+    /**
+     * Changes the stream active count and mActiveClients only.
+     * This does not change the client->active() state or the output descriptor's
+     * global active count.
+     */
     virtual void changeStreamActiveCount(const sp<TrackClientDescriptor>& client, int delta);
             uint32_t streamActiveCount(audio_stream_type_t stream) const
                             { return mActiveCount[stream]; }
+
+    /**
+     * Changes the client->active() state and the output descriptor's global active count,
+     * along with the stream active count and mActiveClients.
+     * The client must be previously added by the base class addClient().
+     */
             void setClientActive(const sp<TrackClientDescriptor>& client, bool active);
 
     bool isActive(uint32_t inPastMs = 0) const;
@@ -76,12 +88,6 @@
     virtual sp<AudioPort> getAudioPort() const { return mPort; }
     virtual void toAudioPort(struct audio_port *port) const;
 
-    using ActiveClientMap = std::map<sp<TrackClientDescriptor>, size_t /* count */>;
-
-    const ActiveClientMap& getActiveClients() const {
-        return mActiveClients;
-    }
-
     audio_module_handle_t getModuleHandle() const;
 
     // implementation of AudioIODescriptorInterface
@@ -89,28 +95,51 @@
     audio_patch_handle_t getPatchHandle() const override;
     void setPatchHandle(audio_patch_handle_t handle) override;
 
-    TrackClientMap& clientsMap() { return mClients; }
     TrackClientVector clientsList(bool activeOnly = false,
         routing_strategy strategy = STRATEGY_NONE, bool preferredDeviceOnly = false) const;
 
-    sp<AudioPort> mPort;
-    audio_devices_t mDevice;                   // current device this output is routed to
+    // override ClientMapHandler to abort when removing a client when active.
+    void removeClient(audio_port_handle_t portId) override {
+        auto client = getClient(portId);
+        LOG_ALWAYS_FATAL_IF(client.get() == nullptr,
+                "%s(%d): nonexistent client portId %d", __func__, mId, portId);
+        // it is possible that when a client is removed, we could remove its
+        // associated active count by calling changeStreamActiveCount(),
+        // but that would be hiding a problem, so we log fatal instead.
+        auto it2 = mActiveClients.find(client);
+        LOG_ALWAYS_FATAL_IF(it2 != mActiveClients.end(),
+                "%s(%d) removing client portId %d which is active (count %zu)",
+                __func__, mId, portId, it2->second);
+        ClientMapHandler<TrackClientDescriptor>::removeClient(portId);
+    }
+
+    using ActiveClientMap = std::map<sp<TrackClientDescriptor>, size_t /* count */>;
+    // required for duplicating thread
+    const ActiveClientMap& getActiveClients() const {
+        return mActiveClients;
+    }
+
+    audio_devices_t mDevice = AUDIO_DEVICE_NONE; // current device this output is routed to
     nsecs_t mStopTime[AUDIO_STREAM_CNT];
-    float mCurVolume[AUDIO_STREAM_CNT];   // current stream volume in dB
-    int mMuteCount[AUDIO_STREAM_CNT];     // mute request counter
+    int mMuteCount[AUDIO_STREAM_CNT];            // mute request counter
     bool mStrategyMutedByDevice[NUM_STRATEGIES]; // strategies muted because of incompatible
                                         // device selection. See checkDeviceMuteStrategies()
-    AudioPolicyClientInterface *mClientInterface;
-    AudioMix *mPolicyMix;             // non NULL when used by a dynamic policy
+    AudioMix *mPolicyMix = nullptr;              // non NULL when used by a dynamic policy
 
 protected:
+    const sp<AudioPort> mPort;
+    AudioPolicyClientInterface * const mClientInterface;
+    float mCurVolume[AUDIO_STREAM_CNT];   // current stream volume in dB
     uint32_t mActiveCount[AUDIO_STREAM_CNT]; // number of streams of each type active on this output
-    uint32_t mGlobalActiveCount;  // non-client-specific active count
-    audio_patch_handle_t mPatchHandle;
-    audio_port_handle_t mId;
-    TrackClientMap mClients;
+    uint32_t mGlobalActiveCount = 0;  // non-client-specific active count
+    audio_patch_handle_t mPatchHandle = AUDIO_PATCH_HANDLE_NONE;
+    audio_port_handle_t mId = AUDIO_PORT_HANDLE_NONE;
+
+    // The ActiveClientMap shows the clients that contribute to the streams counts
+    // and may include upstream clients from a duplicating thread.
+    // Compare with the ClientMap (mClients) which are external AudioTrack clients of the
+    // output descriptor (and do not count internal PatchTracks).
     ActiveClientMap mActiveClients;
-    SimpleLog mLocalLog;
 };
 
 // Audio output driven by a software mixer in audio flinger.
diff --git a/services/audiopolicy/common/managerdefinitions/include/ClientDescriptor.h b/services/audiopolicy/common/managerdefinitions/include/ClientDescriptor.h
index ee231b6..8d7914b 100644
--- a/services/audiopolicy/common/managerdefinitions/include/ClientDescriptor.h
+++ b/services/audiopolicy/common/managerdefinitions/include/ClientDescriptor.h
@@ -163,8 +163,87 @@
 };
 
 typedef std::vector< sp<TrackClientDescriptor> > TrackClientVector;
-typedef std::map< audio_port_handle_t, sp<TrackClientDescriptor> > TrackClientMap;
 typedef std::vector< sp<RecordClientDescriptor> > RecordClientVector;
-typedef std::map< audio_port_handle_t, sp<RecordClientDescriptor> > RecordClientMap;
+
+// A Map that associates a portId with a client (type T)
+// which is either TrackClientDescriptor or RecordClientDescriptor.
+
+template<typename T>
+class ClientMapHandler {
+public:
+    virtual ~ClientMapHandler() = default;
+
+    // Track client management
+    void addClient(const sp<T> &client) {
+        const audio_port_handle_t portId = client->portId();
+        LOG_ALWAYS_FATAL_IF(!mClients.emplace(portId, client).second,
+                "%s(%d): attempting to add client that already exists", __func__, portId);
+    }
+    sp<T> getClient(audio_port_handle_t portId) const {
+        auto it = mClients.find(portId);
+        if (it == mClients.end()) return nullptr;
+        return it->second;
+    }
+    virtual void removeClient(audio_port_handle_t portId) {
+        LOG_ALWAYS_FATAL_IF(mClients.erase(portId) == 0,
+                "%s(%d): client does not exist", __func__, portId);
+    }
+    size_t getClientCount() const {
+        return mClients.size();
+    }
+    String8 dump() const {
+        String8 result;
+        size_t index = 0;
+        for (const auto& client: getClientIterable()) {
+            client->dump(result, 2, index++);
+        }
+        return result;
+    }
+
+    // helper types
+    using ClientMap = std::map<audio_port_handle_t, sp<T>>;
+    using ClientMapIterator = typename ClientMap::const_iterator;  // ClientMap is const qualified
+    class ClientIterable {
+    public:
+        explicit ClientIterable(const ClientMapHandler<T> &ref) : mClientMapHandler(ref) { }
+
+        class iterator {
+        public:
+            // traits
+            using iterator_category = std::forward_iterator_tag;
+            using value_type = sp<T>;
+            using difference_type = ptrdiff_t;
+            using pointer = const sp<T>*;    // Note: const
+            using reference = const sp<T>&;  // Note: const
+
+            // implementation
+            explicit iterator(const ClientMapIterator &it) : mIt(it) { }
+            iterator& operator++()    /* prefix */     { ++mIt; return *this; }
+            reference operator* () const               { return mIt->second; }
+            reference operator->() const               { return mIt->second; } // as if sp<>
+            difference_type operator-(const iterator& rhs) {return mIt - rhs.mIt; }
+            bool operator==(const iterator& rhs) const { return mIt == rhs.mIt; }
+            bool operator!=(const iterator& rhs) const { return mIt != rhs.mIt; }
+        private:
+            ClientMapIterator mIt;
+        };
+
+        iterator begin() const { return iterator{mClientMapHandler.mClients.begin()}; }
+        iterator end() const { return iterator{mClientMapHandler.mClients.end()}; }
+
+    private:
+        const ClientMapHandler<T>& mClientMapHandler; // iterating does not modify map.
+    };
+
+    // return an iterable object that can be used in a range-based-for to enumerate clients.
+    // this iterable does not allow modification, it should be used as a temporary.
+    ClientIterable getClientIterable() const {
+        return ClientIterable{*this};
+    }
+
+private:
+    // ClientMap maps a portId to a client descriptor (both uniquely identify each other).
+    ClientMap mClients;
+};
 
 } // namespace android
diff --git a/services/audiopolicy/common/managerdefinitions/src/AudioInputDescriptor.cpp b/services/audiopolicy/common/managerdefinitions/src/AudioInputDescriptor.cpp
index d9d0d6b..8ef7707 100644
--- a/services/audiopolicy/common/managerdefinitions/src/AudioInputDescriptor.cpp
+++ b/services/audiopolicy/common/managerdefinitions/src/AudioInputDescriptor.cpp
@@ -29,10 +29,8 @@
 
 AudioInputDescriptor::AudioInputDescriptor(const sp<IOProfile>& profile,
                                            AudioPolicyClientInterface *clientInterface)
-    : mIoHandle(0),
-      mDevice(AUDIO_DEVICE_NONE), mPolicyMix(NULL),
-      mProfile(profile), mPatchHandle(AUDIO_PATCH_HANDLE_NONE), mId(0),
-      mClientInterface(clientInterface), mGlobalActiveCount(0)
+    : mProfile(profile)
+    ,  mClientInterface(clientInterface)
 {
     if (profile != NULL) {
         profile->pickAudioProfile(mSamplingRate, mChannelMask, mFormat);
@@ -115,12 +113,12 @@
 
 bool AudioInputDescriptor::isSourceActive(audio_source_t source) const
 {
-    for (const auto &client : mClients) {
-        if (client.second->active() &&
-            ((client.second->source() == source) ||
+    for (const auto &client : getClientIterable()) {
+        if (client->active() &&
+            ((client->source() == source) ||
                 ((source == AUDIO_SOURCE_VOICE_RECOGNITION) &&
-                    (client.second->source() == AUDIO_SOURCE_HOTWORD) &&
-                    client.second->isSoundTrigger()))) {
+                    (client->source() == AUDIO_SOURCE_HOTWORD) &&
+                    client->isSoundTrigger()))) {
             return true;
         }
     }
@@ -132,14 +130,14 @@
     audio_source_t source = AUDIO_SOURCE_DEFAULT;
     int32_t priority = -1;
 
-    for (const auto &client : mClients) {
-        if (activeOnly && !client.second->active() ) {
+    for (const auto &client : getClientIterable()) {
+        if (activeOnly && !client->active() ) {
             continue;
         }
-        int32_t curPriority = source_priority(client.second->source());
+        int32_t curPriority = source_priority(client->source());
         if (curPriority > priority) {
             priority = curPriority;
-            source = client.second->source();
+            source = client->source();
         }
     }
     return source;
@@ -148,10 +146,10 @@
 bool AudioInputDescriptor::isSoundTrigger() const {
     // sound trigger and non sound trigger clients are not mixed on a given input
     // so check only first client
-    if (mClients.size() == 0) {
+    if (getClientCount() == 0) {
         return false;
     }
-    return mClients.cbegin()->second->isSoundTrigger();
+    return getClientIterable().begin()->isSoundTrigger();
 }
 
 audio_patch_handle_t AudioInputDescriptor::getPatchHandle() const
@@ -162,9 +160,9 @@
 void AudioInputDescriptor::setPatchHandle(audio_patch_handle_t handle)
 {
     mPatchHandle = handle;
-    for (const auto &client : mClients) {
-        if (client.second->active()) {
-            updateClientRecordingConfiguration(RECORD_CONFIG_EVENT_START, client.second);
+    for (const auto &client : getClientIterable()) {
+        if (client->active()) {
+            updateClientRecordingConfiguration(RECORD_CONFIG_EVENT_START, client);
         }
     }
 }
@@ -265,8 +263,9 @@
 
 void AudioInputDescriptor::setClientActive(const sp<RecordClientDescriptor>& client, bool active)
 {
-    if (mClients.find(client->portId()) == mClients.end()
-         || active == client->active()) {
+    LOG_ALWAYS_FATAL_IF(getClient(client->portId()) == nullptr,
+        "%s(%d) does not exist on input descriptor", __func__, client->portId());
+    if (active == client->active()) {
         return;
     }
 
@@ -276,7 +275,8 @@
         ALOGW("%s invalid deactivation with globalRefCount %d", __FUNCTION__, mGlobalActiveCount);
         mGlobalActiveCount = 1;
     }
-    mGlobalActiveCount += active ? 1 : -1;
+    const int delta = active ? 1 : -1;
+    mGlobalActiveCount += delta;
 
     if ((oldGlobalActiveCount == 0) && (mGlobalActiveCount > 0)) {
         if ((mPolicyMix != NULL) && ((mPolicyMix->mCbFlags & AudioMix::kCbFlagNotifyActivity) != 0))
@@ -314,9 +314,9 @@
     audio_session_t session)
 {
     RecordClientVector clients;
-    for (const auto &client : mClients) {
-        if (client.second->session() == session) {
-            clients.push_back(client.second);
+    for (const auto &client : getClientIterable()) {
+        if (client->session() == session) {
+            clients.push_back(client);
         }
     }
     return clients;
@@ -326,11 +326,11 @@
                                                      bool preferredDeviceOnly) const
 {
     RecordClientVector clients;
-    for (const auto &client : mClients) {
-        if ((!activeOnly || client.second->active())
-            && (source == AUDIO_SOURCE_DEFAULT || source == client.second->source())
-            && (!preferredDeviceOnly || client.second->hasPreferredDevice())) {
-            clients.push_back(client.second);
+    for (const auto &client : getClientIterable()) {
+        if ((!activeOnly || client->active())
+            && (source == AUDIO_SOURCE_DEFAULT || source == client->source())
+            && (!preferredDeviceOnly || client->hasPreferredDevice())) {
+            clients.push_back(client);
         }
     }
     return clients;
@@ -338,29 +338,16 @@
 
 status_t AudioInputDescriptor::dump(int fd)
 {
-    const size_t SIZE = 256;
-    char buffer[SIZE];
     String8 result;
 
-    snprintf(buffer, SIZE, " ID: %d\n", getId());
-    result.append(buffer);
-    snprintf(buffer, SIZE, " Sampling rate: %d\n", mSamplingRate);
-    result.append(buffer);
-    snprintf(buffer, SIZE, " Format: %d\n", mFormat);
-    result.append(buffer);
-    snprintf(buffer, SIZE, " Channels: %08x\n", mChannelMask);
-    result.append(buffer);
-    snprintf(buffer, SIZE, " Devices %08x\n", mDevice);
-    result.append(buffer);
-
-    write(fd, result.string(), result.size());
-
-    size_t index = 0;
-    result = " AudioRecord clients:\n";
-    for (const auto& client: mClients) {
-        client.second->dump(result, 2, index++);
-    }
-    result.append(" \n");
+    result.appendFormat(" ID: %d\n", getId());
+    result.appendFormat(" Sampling rate: %d\n", mSamplingRate);
+    result.appendFormat(" Format: %d\n", mFormat);
+    result.appendFormat(" Channels: %08x\n", mChannelMask);
+    result.appendFormat(" Devices %08x\n", mDevice);
+    result.append(" AudioRecord Clients:\n");
+    result.append(ClientMapHandler<RecordClientDescriptor>::dump());
+    result.append("\n");
     write(fd, result.string(), result.size());
     return NO_ERROR;
 }
@@ -427,10 +414,8 @@
 {
     for (size_t i = 0; i < size(); i++) {
         sp<AudioInputDescriptor> inputDesc = valueAt(i);
-        for (const auto& client : inputDesc->clientsMap()) {
-            if (client.second->portId() == portId) {
-                return inputDesc;
-            }
+        if (inputDesc->getClient(portId) != nullptr) {
+            return inputDesc;
         }
     }
     return 0;
diff --git a/services/audiopolicy/common/managerdefinitions/src/AudioOutputDescriptor.cpp b/services/audiopolicy/common/managerdefinitions/src/AudioOutputDescriptor.cpp
index ba56b96..03685ab 100644
--- a/services/audiopolicy/common/managerdefinitions/src/AudioOutputDescriptor.cpp
+++ b/services/audiopolicy/common/managerdefinitions/src/AudioOutputDescriptor.cpp
@@ -34,8 +34,8 @@
 
 AudioOutputDescriptor::AudioOutputDescriptor(const sp<AudioPort>& port,
                                              AudioPolicyClientInterface *clientInterface)
-    : mPort(port), mDevice(AUDIO_DEVICE_NONE), mClientInterface(clientInterface),
-      mPolicyMix(NULL), mGlobalActiveCount(0), mPatchHandle(AUDIO_PATCH_HANDLE_NONE), mId(0)
+    : mPort(port)
+    , mClientInterface(clientInterface)
 {
     // clear usage count for all stream types
     for (int i = 0; i < AUDIO_STREAM_CNT; i++) {
@@ -139,20 +139,17 @@
 
 void AudioOutputDescriptor::setClientActive(const sp<TrackClientDescriptor>& client, bool active)
 {
-    if (mClients.find(client->portId()) == mClients.end()
-        || active == client->active()) {
+    LOG_ALWAYS_FATAL_IF(getClient(client->portId()) == nullptr,
+        "%s(%d) does not exist on output descriptor", __func__, client->portId());
 
-        mLocalLog.log("%s(%s): ignored active: %d, current stream count %d",
+    if (active == client->active()) {
+        ALOGW("%s(%s): ignored active: %d, current stream count %d",
                 __func__, client->toShortString().c_str(),
                 active, mActiveCount[client->stream()]);
         return;
     }
-
-    changeStreamActiveCount(client, active ? 1 : -1);
-
-    mLocalLog.log("%s(%s): active: %d, current stream count %d",
-            __func__, client->toShortString().c_str(),
-             active, mActiveCount[client->stream()]);
+    const int delta = active ? 1 : -1;
+    changeStreamActiveCount(client, delta);
 
     // Handle non-client-specific activity ref count
     int32_t oldGlobalActiveCount = mGlobalActiveCount;
@@ -161,7 +158,7 @@
                 __func__, client->toShortString().c_str(), mGlobalActiveCount);
         mGlobalActiveCount = 1;
     }
-    mGlobalActiveCount += active ? 1 : -1;
+    mGlobalActiveCount += delta;
 
     if ((oldGlobalActiveCount == 0) && (mGlobalActiveCount > 0)) {
         if ((mPolicyMix != NULL) && ((mPolicyMix->mCbFlags & AudioMix::kCbFlagNotifyActivity) != 0))
@@ -269,11 +266,11 @@
                                                      bool preferredDeviceOnly) const
 {
     TrackClientVector clients;
-    for (const auto &client : mClients) {
-        if ((!activeOnly || client.second->active())
-            && (strategy == STRATEGY_NONE || strategy == client.second->strategy())
-            && (!preferredDeviceOnly || client.second->hasPreferredDevice())) {
-            clients.push_back(client.second);
+    for (const auto &client : getClientIterable()) {
+        if ((!activeOnly || client->active())
+            && (strategy == STRATEGY_NONE || strategy == client->strategy())
+            && (!preferredDeviceOnly || client->hasPreferredDevice())) {
+            clients.push_back(client);
         }
     }
     return clients;
@@ -281,49 +278,33 @@
 
 status_t AudioOutputDescriptor::dump(int fd)
 {
-    const size_t SIZE = 256;
-    char buffer[SIZE];
     String8 result;
 
-    snprintf(buffer, SIZE, " ID: %d\n", mId);
-    result.append(buffer);
-    snprintf(buffer, SIZE, " Sampling rate: %d\n", mSamplingRate);
-    result.append(buffer);
-    snprintf(buffer, SIZE, " Format: %08x\n", mFormat);
-    result.append(buffer);
-    snprintf(buffer, SIZE, " Channels: %08x\n", mChannelMask);
-    result.append(buffer);
-    snprintf(buffer, SIZE, " Devices %08x\n", device());
-    result.append(buffer);
-    snprintf(buffer, SIZE, " Stream volume activeCount muteCount\n");
-    result.append(buffer);
+    result.appendFormat(" ID: %d\n", mId);
+    result.appendFormat(" Sampling rate: %d\n", mSamplingRate);
+    result.appendFormat(" Format: %08x\n", mFormat);
+    result.appendFormat(" Channels: %08x\n", mChannelMask);
+    result.appendFormat(" Devices: %08x\n", device());
+    result.appendFormat(" Global active count: %u\n", mGlobalActiveCount);
+    result.append(" Stream volume activeCount muteCount\n");
     for (int i = 0; i < (int)AUDIO_STREAM_CNT; i++) {
-        snprintf(buffer, SIZE, " %02d     %.03f     %02d          %02d\n",
+        result.appendFormat(" %02d     %.03f     %02d          %02d\n",
                  i, mCurVolume[i], streamActiveCount((audio_stream_type_t)i), mMuteCount[i]);
-        result.append(buffer);
     }
-
-    result.append(" AudioTrack clients:\n");
-    size_t index = 0;
-    for (const auto& client : mClients) {
-        client.second->dump(result, 2, index++);
-    }
-    result.append(" \n");
-
+    result.append(" AudioTrack Clients:\n");
+    result.append(ClientMapHandler<TrackClientDescriptor>::dump());
+    result.append("\n");
     if (mActiveClients.size() > 0) {
-        result.append(" AudioTrack active clients:\n");
-        index = 0;
+        result.append(" AudioTrack active (stream) clients:\n");
+        size_t index = 0;
         for (const auto& clientPair : mActiveClients) {
             result.appendFormat(" Refcount: %zu", clientPair.second);
             clientPair.first->dump(result, 2, index++);
         }
         result.append(" \n");
     }
-
     write(fd, result.string(), result.size());
 
-    // write local log
-    mLocalLog.dump(fd, "   " /* prefix */, 40 /* lines */);
     return NO_ERROR;
 }
 
@@ -349,14 +330,10 @@
 
 status_t SwAudioOutputDescriptor::dump(int fd)
 {
-    const size_t SIZE = 256;
-    char buffer[SIZE];
     String8 result;
 
-    snprintf(buffer, SIZE, " Latency: %d\n", mLatency);
-    result.append(buffer);
-    snprintf(buffer, SIZE, " Flags %08x\n", mFlags);
-    result.append(buffer);
+    result.appendFormat(" Latency: %d\n", mLatency);
+    result.appendFormat(" Flags %08x\n", mFlags);
     write(fd, result.string(), result.size());
 
     AudioOutputDescriptor::dump(fd);
@@ -808,10 +785,8 @@
 {
     for (size_t i = 0; i < size(); i++) {
         sp<SwAudioOutputDescriptor> outputDesc = valueAt(i);
-        for (const auto& client : outputDesc->clientsMap()) {
-            if (client.second->portId() == portId) {
-                return outputDesc;
-            }
+        if (outputDesc->getClient(portId) != nullptr) {
+            return outputDesc;
         }
     }
     return 0;
diff --git a/services/audiopolicy/common/managerdefinitions/src/ClientDescriptor.cpp b/services/audiopolicy/common/managerdefinitions/src/ClientDescriptor.cpp
index 6e6f507..c237cef 100644
--- a/services/audiopolicy/common/managerdefinitions/src/ClientDescriptor.cpp
+++ b/services/audiopolicy/common/managerdefinitions/src/ClientDescriptor.cpp
@@ -17,6 +17,7 @@
 #define LOG_TAG "APM_ClientDescriptor"
 //#define LOG_NDEBUG 0
 
+#include <sstream>
 #include <utils/Log.h>
 #include <utils/String8.h>
 #include "AudioGain.h"
diff --git a/services/audiopolicy/managerdefault/AudioPolicyManager.cpp b/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
index 4543dd0..6fa0b81 100644
--- a/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
+++ b/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
@@ -910,7 +910,7 @@
                                   getStrategyForAttr(&attributes),
                                   *flags);
     sp<SwAudioOutputDescriptor> outputDesc = mOutputs.valueFor(*output);
-    outputDesc->clientsMap().emplace(*portId, clientDesc);
+    outputDesc->addClient(clientDesc);
 
     ALOGV("  getOutputForAttr() returns output %d selectedDeviceId %d for port ID %d",
           *output, *selectedDeviceId, *portId);
@@ -1320,7 +1320,7 @@
         ALOGW("startOutput() no output for client %d", portId);
         return BAD_VALUE;
     }
-    sp<TrackClientDescriptor> client = outputDesc->clientsMap()[portId];
+    sp<TrackClientDescriptor> client = outputDesc->getClient(portId);
 
     ALOGV("startOutput() output %d, stream %d, session %d",
           outputDesc->mIoHandle, client->stream(), client->session());
@@ -1513,7 +1513,7 @@
         ALOGW("stopOutput() no output for client %d", portId);
         return BAD_VALUE;
     }
-    sp<TrackClientDescriptor> client = outputDesc->clientsMap()[portId];
+    sp<TrackClientDescriptor> client = outputDesc->getClient(portId);
 
     ALOGV("stopOutput() output %d, stream %d, session %d",
           outputDesc->mIoHandle, client->stream(), client->session());
@@ -1614,10 +1614,15 @@
 
     sp<SwAudioOutputDescriptor> outputDesc = mOutputs.getOutputForClient(portId);
     if (outputDesc == 0) {
+        // If an output descriptor is closed due to a device routing change,
+        // then there are race conditions with releaseOutput from tracks
+        // that may be destroyed (with no PlaybackThread) or a PlaybackThread
+        // destroyed shortly thereafter.
+        //
+        // Here we just log a warning, instead of a fatal error.
         ALOGW("releaseOutput() no output for client %d", portId);
         return;
     }
-    sp<TrackClientDescriptor> client = outputDesc->clientsMap()[portId];
 
     ALOGV("releaseOutput() %d", outputDesc->mIoHandle);
 
@@ -1632,10 +1637,12 @@
             mpClientInterface->onAudioPortListUpdate();
         }
     }
-    outputDesc->clientsMap().erase(portId);
+    // stopOutput() needs to be successfully called before releaseOutput()
+    // otherwise there may be inaccurate stream reference counts.
+    // This is checked in outputDesc->removeClient below.
+    outputDesc->removeClient(portId);
 }
 
-
 status_t AudioPolicyManager::getInputForAttr(const audio_attributes_t *attr,
                                              audio_io_handle_t *input,
                                              audio_session_t session,
@@ -1793,7 +1800,7 @@
                                   *attr, *config, requestedDeviceId,
                                   inputSource,flags, isSoundTrigger);
     inputDesc = mInputs.valueFor(*input);
-    inputDesc->clientsMap().emplace(*portId, clientDesc);
+    inputDesc->addClient(clientDesc);
 
     ALOGV("getInputForAttr() returns input %d type %d selectedDeviceId %d for port ID %d",
             *input, *inputType, *selectedDeviceId, *portId);
@@ -1968,7 +1975,7 @@
         return BAD_VALUE;
     }
     audio_io_handle_t input = inputDesc->mIoHandle;
-    sp<RecordClientDescriptor> client = inputDesc->clientsMap()[portId];
+    sp<RecordClientDescriptor> client = inputDesc->getClient(portId);
     if (client->active()) {
         ALOGW("%s input %d client %d already started", __FUNCTION__, input, client->portId());
         return INVALID_OPERATION;
@@ -2137,7 +2144,7 @@
         return BAD_VALUE;
     }
     audio_io_handle_t input = inputDesc->mIoHandle;
-    sp<RecordClientDescriptor> client = inputDesc->clientsMap()[portId];
+    sp<RecordClientDescriptor> client = inputDesc->getClient(portId);
     if (!client->active()) {
         ALOGW("%s input %d client %d already stopped", __FUNCTION__, input, client->portId());
         return INVALID_OPERATION;
@@ -2196,15 +2203,15 @@
         ALOGW("%s no input for client %d", __FUNCTION__, portId);
         return;
     }
-    sp<RecordClientDescriptor> client = inputDesc->clientsMap()[portId];
+    sp<RecordClientDescriptor> client = inputDesc->getClient(portId);
     audio_io_handle_t input = inputDesc->mIoHandle;
 
     ALOGV("%s %d", __FUNCTION__, input);
 
-    inputDesc->clientsMap().erase(portId);
+    inputDesc->removeClient(portId);
 
-    if (inputDesc->clientsMap().size() > 0) {
-        ALOGV("%s %zu clients remaining", __FUNCTION__, inputDesc->clientsMap().size());
+    if (inputDesc->getClientCount() > 0) {
+        ALOGV("%s(%d) %zu clients remaining", __func__, portId, inputDesc->getClientCount());
         return;
     }
 
@@ -3311,11 +3318,10 @@
     SortedVector<routing_strategy> affectedStrategies;
     for (size_t i = 0; i < mOutputs.size(); i++) {
         sp<AudioOutputDescriptor> outputDesc = mOutputs.valueAt(i);
-        TrackClientMap clients = outputDesc->clientsMap();
-        for (const auto& client : clients) {
-            if (client.second->hasPreferredDevice() && client.second->uid() == uid) {
-                client.second->setPreferredDeviceId(AUDIO_PORT_HANDLE_NONE);
-                affectedStrategies.add(getStrategy(client.second->stream()));
+        for (const auto& client : outputDesc->getClientIterable()) {
+            if (client->hasPreferredDevice() && client->uid() == uid) {
+                client->setPreferredDeviceId(AUDIO_PORT_HANDLE_NONE);
+                affectedStrategies.add(getStrategy(client->stream()));
             }
         }
     }
@@ -3328,11 +3334,10 @@
     SortedVector<audio_source_t> affectedSources;
     for (size_t i = 0; i < mInputs.size(); i++) {
         sp<AudioInputDescriptor> inputDesc = mInputs.valueAt(i);
-        RecordClientMap clients = inputDesc->clientsMap();
-        for (const auto& client : clients) {
-            if (client.second->hasPreferredDevice() && client.second->uid() == uid) {
-                client.second->setPreferredDeviceId(AUDIO_PORT_HANDLE_NONE);
-                affectedSources.add(client.second->source());
+        for (const auto& client : inputDesc->getClientIterable()) {
+            if (client->hasPreferredDevice() && client->uid() == uid) {
+                client->setPreferredDeviceId(AUDIO_PORT_HANDLE_NONE);
+                affectedSources.add(client->source());
             }
         }
     }