From 6ee09f04577dc07e3242c1214fcb6d5501b29a77 Mon Sep 17 00:00:00 2001
From: Andy Hung <hunga@google.com>
Date: Tue, 12 Jan 2021 14:54:10 -0800
Subject: [PATCH] StreamOut: use atomic_sp<> for callback thread safety

Test: see bug
Test: basic audio works
Bug: 177278988
Change-Id: Id1f5036e36b2a3b3e71e0b0ec548d4c65a972b50
---
 audio/core/all-versions/default/Android.bp                  | 1 +
 audio/core/all-versions/default/StreamOut.cpp               | 6 +++---
 .../all-versions/default/include/core/default/StreamOut.h   | 3 ++-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/audio/core/all-versions/default/Android.bp b/audio/core/all-versions/default/Android.bp
index e0f086094c..c75c779a91 100644
--- a/audio/core/all-versions/default/Android.bp
+++ b/audio/core/all-versions/default/Android.bp
@@ -51,6 +51,7 @@ cc_defaults {
         "libaudio_system_headers",
         "libhardware_headers",
         "libmedia_headers",
+        "libmediautils_headers",
     ],
 
     export_header_lib_headers: [
diff --git a/audio/core/all-versions/default/StreamOut.cpp b/audio/core/all-versions/default/StreamOut.cpp
index ffd3b6b856..1fb72671bb 100644
--- a/audio/core/all-versions/default/StreamOut.cpp
+++ b/audio/core/all-versions/default/StreamOut.cpp
@@ -163,7 +163,7 @@ StreamOut::~StreamOut() {
         status_t status = EventFlag::deleteEventFlag(&mEfGroup);
         ALOGE_IF(status, "write MQ event flag deletion error: %s", strerror(-status));
     }
-    mCallback.clear();
+    mCallback = nullptr;
 #if MAJOR_VERSION <= 5
     mDevice->closeOutputStream(mStream);
     // Closing the output stream in the HAL waits for the callback to finish,
@@ -462,7 +462,7 @@ Return<Result> StreamOut::setCallback(const sp<IStreamOutCallback>& callback) {
 
 Return<Result> StreamOut::clearCallback() {
     if (mStream->set_callback == NULL) return Result::NOT_SUPPORTED;
-    mCallback.clear();
+    mCallback = nullptr;
     return Result::OK;
 }
 
@@ -477,7 +477,7 @@ int StreamOut::asyncCallback(stream_callback_event_t event, void*, void* cookie)
     // It's correct to hold an sp<> to callback because the reference
     // in the StreamOut instance can be cleared in the meantime. There is
     // no difference on which thread to run IStreamOutCallback's destructor.
-    sp<IStreamOutCallback> callback = self->mCallback;
+    sp<IStreamOutCallback> callback = self->mCallback.load();
     if (callback.get() == nullptr) return 0;
     ALOGV("asyncCallback() event %d", event);
     Return<void> result;
diff --git a/audio/core/all-versions/default/include/core/default/StreamOut.h b/audio/core/all-versions/default/include/core/default/StreamOut.h
index b8e8515739..c246ef3e4c 100644
--- a/audio/core/all-versions/default/include/core/default/StreamOut.h
+++ b/audio/core/all-versions/default/include/core/default/StreamOut.h
@@ -29,6 +29,7 @@
 #include <fmq/MessageQueue.h>
 #include <hidl/MQDescriptor.h>
 #include <hidl/Status.h>
+#include <mediautils/Synchronization.h>
 #include <utils/Thread.h>
 
 namespace android {
@@ -158,7 +159,7 @@ struct StreamOut : public IStreamOut {
     audio_stream_out_t* mStream;
     const sp<Stream> mStreamCommon;
     const sp<StreamMmap<audio_stream_out_t>> mStreamMmap;
-    sp<IStreamOutCallback> mCallback;  // Callback for non-blocking write and drain
+    mediautils::atomic_sp<IStreamOutCallback> mCallback;  // for non-blocking write and drain
 #if MAJOR_VERSION >= 6
     sp<IStreamOutEventCallback> mEventCallback;
 #endif