blast: fix registering callbacks
This is a better fix for b/134194071 than ag/7998544. The previous
patch only protected the call to notify the binder thread to send a
callback.
This patch has both a start registration and end registration call.
During that time, the transaction callback cannot be sent. This is
closer to a long term fix for the bug.
Bug: 134194071
Test: Switch between front and back cameras to make sure the app
doesn't crash.
Change-Id: I2d20c13cc1c8d13e5a1340dfaa8cbbaa4d3a30ab
diff --git a/services/surfaceflinger/TransactionCompletedThread.cpp b/services/surfaceflinger/TransactionCompletedThread.cpp
index 5cf8eb1..0806f2a 100644
--- a/services/surfaceflinger/TransactionCompletedThread.cpp
+++ b/services/surfaceflinger/TransactionCompletedThread.cpp
@@ -75,14 +75,15 @@
mThread = std::thread(&TransactionCompletedThread::threadMain, this);
}
-status_t TransactionCompletedThread::addCallback(const sp<ITransactionCompletedListener>& listener,
- const std::vector<CallbackId>& callbackIds) {
+status_t TransactionCompletedThread::startRegistration(const ListenerCallbacks& listenerCallbacks) {
std::lock_guard lock(mMutex);
if (!mRunning) {
ALOGE("cannot add callback because the callback thread isn't running");
return BAD_VALUE;
}
+ auto& [listener, callbackIds] = listenerCallbacks;
+
if (mCompletedTransactions.count(listener) == 0) {
status_t err = IInterface::asBinder(listener)->linkToDeath(mDeathRecipient);
if (err != NO_ERROR) {
@@ -91,11 +92,41 @@
}
}
+ mRegisteringTransactions.insert(listenerCallbacks);
+
auto& transactionStatsDeque = mCompletedTransactions[listener];
transactionStatsDeque.emplace_back(callbackIds);
+
return NO_ERROR;
}
+status_t TransactionCompletedThread::endRegistration(const ListenerCallbacks& listenerCallbacks) {
+ std::lock_guard lock(mMutex);
+ if (!mRunning) {
+ ALOGE("cannot add callback because the callback thread isn't running");
+ return BAD_VALUE;
+ }
+
+ auto itr = mRegisteringTransactions.find(listenerCallbacks);
+ if (itr == mRegisteringTransactions.end()) {
+ ALOGE("cannot end a registration that does not exist");
+ return BAD_VALUE;
+ }
+
+ mRegisteringTransactions.erase(itr);
+
+ return NO_ERROR;
+}
+
+bool TransactionCompletedThread::isRegisteringTransaction(
+ const sp<ITransactionCompletedListener>& transactionListener,
+ const std::vector<CallbackId>& callbackIds) {
+ ListenerCallbacks listenerCallbacks(transactionListener, callbackIds);
+
+ auto itr = mRegisteringTransactions.find(listenerCallbacks);
+ return itr != mRegisteringTransactions.end();
+}
+
status_t TransactionCompletedThread::registerPendingCallbackHandle(
const sp<CallbackHandle>& handle) {
std::lock_guard lock(mMutex);
@@ -105,7 +136,7 @@
}
// If we can't find the transaction stats something has gone wrong. The client should call
- // addCallback before trying to register a pending callback handle.
+ // startRegistration before trying to register a pending callback handle.
TransactionStats* transactionStats;
status_t err = findTransactionStats(handle->listener, handle->callbackIds, &transactionStats);
if (err != NO_ERROR) {
@@ -117,7 +148,7 @@
return NO_ERROR;
}
-status_t TransactionCompletedThread::addPresentedCallbackHandles(
+status_t TransactionCompletedThread::finalizePendingCallbackHandles(
const std::deque<sp<CallbackHandle>>& handles) {
std::lock_guard lock(mMutex);
if (!mRunning) {
@@ -158,7 +189,7 @@
return NO_ERROR;
}
-status_t TransactionCompletedThread::addUnpresentedCallbackHandle(
+status_t TransactionCompletedThread::registerUnpresentedCallbackHandle(
const sp<CallbackHandle>& handle) {
std::lock_guard lock(mMutex);
if (!mRunning) {
@@ -189,7 +220,7 @@
status_t TransactionCompletedThread::addCallbackHandle(const sp<CallbackHandle>& handle) {
// If we can't find the transaction stats something has gone wrong. The client should call
- // addCallback before trying to add a presnted callback handle.
+ // startRegistration before trying to add a callback handle.
TransactionStats* transactionStats;
status_t err = findTransactionStats(handle->listener, handle->callbackIds, &transactionStats);
if (err != NO_ERROR) {
@@ -233,6 +264,13 @@
while (transactionStatsItr != transactionStatsDeque.end()) {
auto& transactionStats = *transactionStatsItr;
+ // If this transaction is still registering, it is not safe to send a callback
+ // because there could be surface controls that haven't been added to
+ // transaction stats or mPendingTransactions.
+ if (isRegisteringTransaction(listener, transactionStats.callbackIds)) {
+ break;
+ }
+
// If we are still waiting on the callback handles for this transaction, stop
// here because all transaction callbacks for the same listener must come in order
auto pendingTransactions = mPendingTransactions.find(listener);