commit 6e6a0b04fdeaa0a134e245d3baab9d1395ac6ed2
author Patrick Rohr <prohr@google.com> Mon Nov 29 14:53:17 2021 +0100
committer Patrick Rohr <prohr@google.com> Mon Nov 29 19:02:12 2021 +0100
tree eb9e33cbb4a599163ad5ce01e66765cb2819f73e
parent 82a564515c8131462ca27a09b387cebbe4ce3af3

fix mFilterEvents locking

mFilterEventsLock was not always being held when mFilterEvents is
modified.

Test: atest VtsHalTvTunerTargetTest
Bug: 183057734
Change-Id: I5b6bdb3bb8674d6da89665182ce1694f04dc64b2

tv/tuner/aidl/default/Filter.cpp

diff --git a/tv/tuner/aidl/default/Filter.cpp b/tv/tuner/aidl/default/Filter.cpp
index 5f61c8d..f6436cd 100644
--- a/tv/tuner/aidl/default/Filter.cpp
+++ b/tv/tuner/aidl/default/Filter.cpp
@@ -542,7 +542,9 @@
     // For the first time of filter output, implementation needs to send the filter
     // Event Callback without waiting for the DATA_CONSUMED to init the process.
     while (mFilterThreadRunning) {
+        std::unique_lock<std::mutex> lock(mFilterEventsLock);
         if (mFilterEvents.size() == 0) {
+            lock.unlock();
             if (DEBUG_FILTER) {
                 ALOGD("[Filter] wait for filter data output.");
             }
@@ -559,6 +561,7 @@
                 mConfigured = false;
             }
 
+            // lock is still being held
             for (auto&& event : mFilterEvents) {
                 mCallbackScheduler.onFilterEvent(std::move(event));
             }
@@ -741,7 +744,6 @@
 }
 
 ::ndk::ScopedAStatus Filter::startPesFilterHandler() {
-    std::lock_guard<std::mutex> lock(mFilterEventsLock);
     if (mFilterOutput.empty()) {
         return ::ndk::ScopedAStatus::ok();
     }
@@ -796,7 +798,11 @@
             ALOGD("[Filter] assembled pes data length %d", pesEvent.dataLength);
         }
 
-        mFilterEvents.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::pes>(pesEvent));
+        {
+            std::lock_guard<std::mutex> lock(mFilterEventsLock);
+            mFilterEvents.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::pes>(pesEvent));
+        }
+
         mPesOutput.clear();
     }
 
@@ -811,7 +817,6 @@
 }
 
 ::ndk::ScopedAStatus Filter::startMediaFilterHandler() {
-    std::lock_guard<std::mutex> lock(mFilterEventsLock);
     if (mFilterOutput.empty()) {
         return ::ndk::ScopedAStatus::ok();
     }
@@ -900,7 +905,12 @@
             .firstMbInSlice = 0,  // random address
     };
 
-    mFilterEvents.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::tsRecord>(recordEvent));
+    {
+        std::lock_guard<std::mutex> lock(mFilterEventsLock);
+        mFilterEvents.push_back(
+                DemuxFilterEvent::make<DemuxFilterEvent::Tag::tsRecord>(recordEvent));
+    }
+
     mRecordFilterOutput.clear();
     return ::ndk::ScopedAStatus::ok();
 }
@@ -918,7 +928,6 @@
 bool Filter::writeSectionsAndCreateEvent(vector<int8_t>& data) {
     // TODO check how many sections has been read
     ALOGD("[Filter] section handler");
-    std::lock_guard<std::mutex> lock(mFilterEventsLock);
     if (!writeDataToFilterMQ(data)) {
         return false;
     }
@@ -930,7 +939,12 @@
             .sectionNum = 1,
             .dataLength = static_cast<int32_t>(data.size()),
     };
-    mFilterEvents.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::section>(secEvent));
+
+    {
+        std::lock_guard<std::mutex> lock(mFilterEventsLock);
+        mFilterEvents.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::section>(secEvent));
+    }
+
     return true;
 }
 
@@ -1028,7 +1042,11 @@
         mediaEvent.pts = mPts;
         mPts = 0;
     }
-    mFilterEvents.push_back(std::move(event));
+
+    {
+        std::lock_guard<std::mutex> lock(mFilterEventsLock);
+        mFilterEvents.push_back(std::move(event));
+    }
 
     // Clear and log
     native_handle_close(nativeHandle);
@@ -1068,7 +1086,11 @@
         mediaEvent.pts = mPts;
         mPts = 0;
     }
-    mFilterEvents.push_back(std::move(event));
+
+    {
+        std::lock_guard<std::mutex> lock(mFilterEventsLock);
+        mFilterEvents.push_back(std::move(event));
+    }
 
     mSharedAvMemOffset += output.size();