commit 14aabae62a12de0a26bddf0a6ad7bad603b7824e
author Atneya Nair <atneya@google.com> Tue Nov 30 17:36:24 2021 -0500
committer Atneya Nair <atneya@google.com> Tue Feb 08 17:56:47 2022 +0000
tree a0079c3fc5431d76918317451c7d3bbe51904662
parent 58d6ef8088ac8f925d4b972a57623f6fe47b133a

Fix mCallback race. Protect repeat set.

Protecting mCallback with a lock for thread correctness.
Check if we call set twice, this breaks invariants.

Test: Compiles
Change-Id: I81cfbe63308ff1202da6a6b935163314895b66f8

media/libaudioclient/AudioTrack.cpp

diff --git a/media/libaudioclient/AudioTrack.cpp b/media/libaudioclient/AudioTrack.cpp
index de14e1c..4bc61f5 100644
--- a/media/libaudioclient/AudioTrack.cpp
+++ b/media/libaudioclient/AudioTrack.cpp
@@ -537,6 +537,8 @@
         float maxRequiredSpeed,
         audio_port_handle_t selectedDeviceId)
 {
+    LOG_ALWAYS_FATAL_IF(mInitialized, "%s: should not be called twice", __func__);
+    mInitialized = true;
     status_t status;
     uint32_t channelCount;
     pid_t callingPid;
@@ -1591,12 +1593,12 @@
 
 status_t AudioTrack::setMarkerPosition(uint32_t marker)
 {
+    AutoMutex lock(mLock);
     // The only purpose of setting marker position is to get a callback
-    if (!mCallback.promote() || isOffloadedOrDirect()) {
+    if (!mCallback.promote() || isOffloadedOrDirect_l()) {
         return INVALID_OPERATION;
     }
 
-    AutoMutex lock(mLock);
     mMarkerPosition = marker;
     mMarkerReached = false;
 
@@ -1624,12 +1626,12 @@
 
 status_t AudioTrack::setPositionUpdatePeriod(uint32_t updatePeriod)
 {
+    AutoMutex lock(mLock);
     // The only purpose of setting position update period is to get a callback
-    if (!mCallback.promote() || isOffloadedOrDirect()) {
+    if (!mCallback.promote() || isOffloadedOrDirect_l()) {
         return INVALID_OPERATION;
     }
 
-    AutoMutex lock(mLock);
     mNewPosition = updateAndGetPosition_l() + updatePeriod;
     mUpdatePeriod = updatePeriod;