Merge "Ensure reportFrameMetrics not being called on deleted instance" into sc-dev

2 files changed, 15 insertions, 6 deletions

diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 96104376d1..8af21d3c5d 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -182,12 +182,12 @@ CallbackId TransactionCompletedListener::addCallbackFunction(
 
 void TransactionCompletedListener::addJankListener(const sp<JankDataListener>& listener,
                                                    sp<SurfaceControl> surfaceControl) {
-    std::lock_guard<std::mutex> lock(mMutex);
+    std::scoped_lock<std::recursive_mutex> lock(mJankListenerMutex);
     mJankListeners.insert({surfaceControl->getHandle(), listener});
 }
 
 void TransactionCompletedListener::removeJankListener(const sp<JankDataListener>& listener) {
-    std::lock_guard<std::mutex> lock(mMutex);
+    std::scoped_lock<std::recursive_mutex> lock(mJankListenerMutex);
     for (auto it = mJankListeners.begin(); it != mJankListeners.end();) {
         if (it->second == listener) {
             it = mJankListeners.erase(it);
@@ -242,7 +242,6 @@ void TransactionCompletedListener::addSurfaceControlToCallbacks(
 
 void TransactionCompletedListener::onTransactionCompleted(ListenerStats listenerStats) {
     std::unordered_map<CallbackId, CallbackTranslation, CallbackIdHash> callbacksMap;
-    std::multimap<sp<IBinder>, sp<JankDataListener>> jankListenersMap;
     std::multimap<sp<IBinder>, SurfaceStatsCallbackEntry> surfaceListeners;
     {
         std::lock_guard<std::mutex> lock(mMutex);
@@ -259,7 +258,6 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener
          * sp<SurfaceControl> that could possibly exist for the callbacks.
          */
         callbacksMap = mCallbacks;
-        jankListenersMap = mJankListeners;
         surfaceListeners = mSurfaceStatsListeners;
         for (const auto& transactionStats : listenerStats.transactionStats) {
             for (auto& callbackId : transactionStats.callbackIds) {
@@ -349,7 +347,12 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener
             }
 
             if (surfaceStats.jankData.empty()) continue;
-            auto jankRange = jankListenersMap.equal_range(surfaceStats.surfaceControl);
+
+            // Acquire jank listener lock such that we guarantee that after calling unregister,
+            // there won't be any further callback.
+            std::scoped_lock<std::recursive_mutex> lock(mJankListenerMutex);
+            auto copy = mJankListeners;
+            auto jankRange = copy.equal_range(surfaceStats.surfaceControl);
             for (auto it = jankRange.first; it != jankRange.second; it++) {
                 it->second->onJankDataAvailable(surfaceStats.jankData);
             }
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 62a782fd3e..28b4a3724e 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -652,6 +652,9 @@ class TransactionCompletedListener : public BnTransactionCompletedListener {
 
     std::mutex mMutex;
 
+    // This lock needs to be recursive so we can unregister a callback from within that callback.
+    std::recursive_mutex mJankListenerMutex;
+
     bool mListening GUARDED_BY(mMutex) = false;
 
     int64_t mCallbackIdCounter GUARDED_BY(mMutex) = 1;
@@ -674,7 +677,10 @@ class TransactionCompletedListener : public BnTransactionCompletedListener {
 
     std::unordered_map<CallbackId, CallbackTranslation, CallbackIdHash> mCallbacks
             GUARDED_BY(mMutex);
-    std::multimap<sp<IBinder>, sp<JankDataListener>> mJankListeners GUARDED_BY(mMutex);
+
+    // This is protected by mJankListenerMutex, but GUARDED_BY isn't supported for
+    // std::recursive_mutex
+    std::multimap<sp<IBinder>, sp<JankDataListener>> mJankListeners;
     std::unordered_map<uint64_t /* graphicsBufferId */, ReleaseBufferCallback>
             mReleaseBufferCallbacks GUARDED_BY(mMutex);
     std::multimap<sp<IBinder>, SurfaceStatsCallbackEntry>