From 0e7bcae462e05dbdf4ef797365f9414437914bd7 Mon Sep 17 00:00:00 2001
From: Mikhail Naganov <mnaganov@google.com>
Date: Tue, 12 Sep 2023 12:40:43 -0700
Subject: [PATCH] audio: Mitigate double receiving of the "exit" command

In rare cases, a worker thread from a new stream created
via StreamSwitcher can read again from the command FMQ
the "exit" command which was sent to the worker of
the previous stream.

The underlying reason for that has to be investigated.
For now, mitigate the issue by salting the cookie of
the "exit" command with the worker's TID.

Bug: 300130515
Test: atest VtsHalAudioCoreTargetTest
Change-Id: Ie7d2e847e8b39414ffd31afd64e32d4c9a292c03
---
 audio/aidl/default/Stream.cpp                 | 16 +++++++++++++---
 audio/aidl/default/include/core-impl/Stream.h |  1 +
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/audio/aidl/default/Stream.cpp b/audio/aidl/default/Stream.cpp
index af89f5fce8..f7298c0286 100644
--- a/audio/aidl/default/Stream.cpp
+++ b/audio/aidl/default/Stream.cpp
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#include <pthread.h>
+
 #define LOG_TAG "AHAL_Stream"
 #include <android-base/logging.h>
 #include <android/binder_ibinder_platform.h>
@@ -94,6 +96,14 @@ void StreamContext::reset() {
     mDataMQ.reset();
 }
 
+pid_t StreamWorkerCommonLogic::getTid() const {
+#if defined(__ANDROID__)
+    return pthread_gettid_np(pthread_self());
+#else
+    return 0;
+#endif
+}
+
 std::string StreamWorkerCommonLogic::init() {
     if (mContext->getCommandMQ() == nullptr) return "Command MQ is null";
     if (mContext->getReplyMQ() == nullptr) return "Reply MQ is null";
@@ -164,7 +174,7 @@ StreamInWorkerLogic::Status StreamInWorkerLogic::cycle() {
     switch (command.getTag()) {
         case Tag::halReservedExit:
             if (const int32_t cookie = command.get<Tag::halReservedExit>();
-                cookie == mContext->getInternalCommandCookie()) {
+                cookie == (mContext->getInternalCommandCookie() ^ getTid())) {
                 mDriver->shutdown();
                 setClosed();
                 // This is an internal command, no need to reply.
@@ -384,7 +394,7 @@ StreamOutWorkerLogic::Status StreamOutWorkerLogic::cycle() {
     switch (command.getTag()) {
         case Tag::halReservedExit:
             if (const int32_t cookie = command.get<Tag::halReservedExit>();
-                cookie == mContext->getInternalCommandCookie()) {
+                cookie == (mContext->getInternalCommandCookie() ^ getTid())) {
                 mDriver->shutdown();
                 setClosed();
                 // This is an internal command, no need to reply.
@@ -717,7 +727,7 @@ void StreamCommonImpl::stopWorker() {
     if (auto commandMQ = mContext.getCommandMQ(); commandMQ != nullptr) {
         LOG(DEBUG) << __func__ << ": asking the worker to exit...";
         auto cmd = StreamDescriptor::Command::make<StreamDescriptor::Command::Tag::halReservedExit>(
-                mContext.getInternalCommandCookie());
+                mContext.getInternalCommandCookie() ^ mWorker->getTid());
         // Note: never call 'pause' and 'resume' methods of StreamWorker
         // in the HAL implementation. These methods are to be used by
         // the client side only. Preventing the worker loop from running
diff --git a/audio/aidl/default/include/core-impl/Stream.h b/audio/aidl/default/include/core-impl/Stream.h
index a02655f1b7..88fddec233 100644
--- a/audio/aidl/default/include/core-impl/Stream.h
+++ b/audio/aidl/default/include/core-impl/Stream.h
@@ -223,6 +223,7 @@ class StreamWorkerCommonLogic : public ::android::hardware::audio::common::Strea
         : mContext(context),
           mDriver(driver),
           mTransientStateDelayMs(context->getTransientStateDelayMs()) {}
+    pid_t getTid() const;
     std::string init() override;
     void populateReply(StreamDescriptor::Reply* reply, bool isConnected) const;
     void populateReplyWrongState(StreamDescriptor::Reply* reply,