From 1850779bc4d7808e4d3171035ce2e027b6e36ddc Mon Sep 17 00:00:00 2001
From: Mikhail Naganov <mnaganov@google.com>
Date: Fri, 12 Jan 2024 13:48:21 -0800
Subject: [PATCH] audio: Fix handling of a thread exit command with a bad
 cookie

In case when the command was sent by the HAL itself (from another
thread), the worker thread must not post a reply. The only case
when a reply needs to be posted is in the case when the command
was sent from a VTS test. This case is identified by the fact that
the cookie has value '0'.

Bug: 300181540
Test: atest VtsHalAudioCoreTargetTest
Change-Id: Ifeb0722b5cf7346a694c5a938f6b324f5fa825f1
---
 audio/aidl/default/Stream.cpp                 | 30 +++++++++++--------
 audio/aidl/default/include/core-impl/Stream.h |  2 +-
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/audio/aidl/default/Stream.cpp b/audio/aidl/default/Stream.cpp
index a805b872cd..cf0870e2d4 100644
--- a/audio/aidl/default/Stream.cpp
+++ b/audio/aidl/default/Stream.cpp
@@ -180,17 +180,20 @@ StreamInWorkerLogic::Status StreamInWorkerLogic::cycle() {
     StreamDescriptor::Reply reply{};
     reply.status = STATUS_BAD_VALUE;
     switch (command.getTag()) {
-        case Tag::halReservedExit:
-            if (const int32_t cookie = command.get<Tag::halReservedExit>();
-                cookie == (mContext->getInternalCommandCookie() ^ getTid())) {
+        case Tag::halReservedExit: {
+            const int32_t cookie = command.get<Tag::halReservedExit>();
+            if (cookie == (mContext->getInternalCommandCookie() ^ getTid())) {
                 mDriver->shutdown();
                 setClosed();
-                // This is an internal command, no need to reply.
-                return Status::EXIT;
             } else {
                 LOG(WARNING) << __func__ << ": EXIT command has a bad cookie: " << cookie;
             }
-            break;
+            if (cookie != 0) {  // This is an internal command, no need to reply.
+                return Status::EXIT;
+            } else {
+                break;
+            }
+        }
         case Tag::getStatus:
             populateReply(&reply, mIsConnected);
             break;
@@ -400,17 +403,20 @@ StreamOutWorkerLogic::Status StreamOutWorkerLogic::cycle() {
     reply.status = STATUS_BAD_VALUE;
     using Tag = StreamDescriptor::Command::Tag;
     switch (command.getTag()) {
-        case Tag::halReservedExit:
-            if (const int32_t cookie = command.get<Tag::halReservedExit>();
-                cookie == (mContext->getInternalCommandCookie() ^ getTid())) {
+        case Tag::halReservedExit: {
+            const int32_t cookie = command.get<Tag::halReservedExit>();
+            if (cookie == (mContext->getInternalCommandCookie() ^ getTid())) {
                 mDriver->shutdown();
                 setClosed();
-                // This is an internal command, no need to reply.
-                return Status::EXIT;
             } else {
                 LOG(WARNING) << __func__ << ": EXIT command has a bad cookie: " << cookie;
             }
-            break;
+            if (cookie != 0) {  // This is an internal command, no need to reply.
+                return Status::EXIT;
+            } else {
+                break;
+            }
+        }
         case Tag::getStatus:
             populateReply(&reply, mIsConnected);
             break;
diff --git a/audio/aidl/default/include/core-impl/Stream.h b/audio/aidl/default/include/core-impl/Stream.h
index aa9fb19a5b..21e63f9fef 100644
--- a/audio/aidl/default/include/core-impl/Stream.h
+++ b/audio/aidl/default/include/core-impl/Stream.h
@@ -90,7 +90,7 @@ class StreamContext {
                   std::weak_ptr<sounddose::StreamDataProcessorInterface> streamDataProcessor,
                   DebugParameters debugParameters)
         : mCommandMQ(std::move(commandMQ)),
-          mInternalCommandCookie(std::rand()),
+          mInternalCommandCookie(std::rand() | 1 /* make sure it's not 0 */),
           mReplyMQ(std::move(replyMQ)),
           mFormat(format),
           mChannelLayout(channelLayout),