Ensure reportFrameMetrics not being called on deleted instance
Since onSurfaceStatsAvailable gets called on binder-thread, we
need to ensure that instance doesn't get released while
onSurfaceStatsAvailable is calling reportFrameMetrics.
We do this by introducing a lock for register/unregister/callback,
such than when unregister completes, there won't be any "hanging"
callback anymore such that the callback can't be called anymore
on deleted instances.
Test: Boots
Bug: 188934435
Change-Id: Ifc5357bd181e0cd065cdecd0188836a35f87b3e2
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 768f572..1057a51 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -210,13 +210,13 @@
void TransactionCompletedListener::addSurfaceStatsListener(void* context, void* cookie,
sp<SurfaceControl> surfaceControl, SurfaceStatsCallback listener) {
- std::lock_guard<std::mutex> lock(mMutex);
+ std::scoped_lock<std::recursive_mutex> lock(mSurfaceStatsListenerMutex);
mSurfaceStatsListeners.insert({surfaceControl->getHandle(),
SurfaceStatsCallbackEntry(context, cookie, listener)});
}
void TransactionCompletedListener::removeSurfaceStatsListener(void* context, void* cookie) {
- std::lock_guard<std::mutex> lock(mMutex);
+ std::scoped_lock<std::recursive_mutex> lock(mSurfaceStatsListenerMutex);
for (auto it = mSurfaceStatsListeners.begin(); it != mSurfaceStatsListeners.end();) {
auto [itContext, itCookie, itListener] = it->second;
if (itContext == context && itCookie == cookie) {
@@ -242,7 +242,6 @@
void TransactionCompletedListener::onTransactionCompleted(ListenerStats listenerStats) {
std::unordered_map<CallbackId, CallbackTranslation, CallbackIdHash> callbacksMap;
- std::multimap<sp<IBinder>, SurfaceStatsCallbackEntry> surfaceListeners;
{
std::lock_guard<std::mutex> lock(mMutex);
@@ -258,7 +257,6 @@
* sp<SurfaceControl> that could possibly exist for the callbacks.
*/
callbacksMap = mCallbacks;
- surfaceListeners = mSurfaceStatsListeners;
for (const auto& transactionStats : listenerStats.transactionStats) {
for (auto& callbackId : transactionStats.callbackIds) {
mCallbacks.erase(callbackId);
@@ -338,11 +336,17 @@
surfaceControlStats);
}
for (const auto& surfaceStats : transactionStats.surfaceStats) {
- auto listenerRange = surfaceListeners.equal_range(surfaceStats.surfaceControl);
- for (auto it = listenerRange.first; it != listenerRange.second; it++) {
- auto entry = it->second;
- entry.callback(entry.context, transactionStats.latchTime,
- transactionStats.presentFence, surfaceStats);
+ {
+ // Acquire surface stats listener lock such that we guarantee that after calling
+ // unregister, there won't be any further callback.
+ std::scoped_lock<std::recursive_mutex> lock(mSurfaceStatsListenerMutex);
+ auto listenerRange = mSurfaceStatsListeners.equal_range(
+ surfaceStats.surfaceControl);
+ for (auto it = listenerRange.first; it != listenerRange.second; it++) {
+ auto entry = it->second;
+ entry.callback(entry.context, transactionStats.latchTime,
+ transactionStats.presentFence, surfaceStats);
+ }
}
if (surfaceStats.jankData.empty()) continue;