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.h b/services/surfaceflinger/TransactionCompletedThread.h
index 21e2678..b821350 100644
--- a/services/surfaceflinger/TransactionCompletedThread.h
+++ b/services/surfaceflinger/TransactionCompletedThread.h
@@ -21,6 +21,7 @@
#include <mutex>
#include <thread>
#include <unordered_map>
+#include <unordered_set>
#include <android-base/thread_annotations.h>
@@ -30,6 +31,12 @@
namespace android {
+struct ITransactionCompletedListenerHash {
+ std::size_t operator()(const sp<ITransactionCompletedListener>& listener) const {
+ return std::hash<IBinder*>{}((listener) ? IInterface::asBinder(listener).get() : nullptr);
+ }
+};
+
struct CallbackIdsHash {
// CallbackId vectors have several properties that let us get away with this simple hash.
// 1) CallbackIds are never 0 so if something has gone wrong and our CallbackId vector is
@@ -42,6 +49,22 @@
}
};
+struct ListenerCallbacksHash {
+ std::size_t HashCombine(size_t value1, size_t value2) const {
+ return value1 ^ (value2 + 0x9e3779b9 + (value1 << 6) + (value1 >> 2));
+ }
+
+ std::size_t operator()(const ListenerCallbacks& listenerCallbacks) const {
+ struct ITransactionCompletedListenerHash listenerHasher;
+ struct CallbackIdsHash callbackIdsHasher;
+
+ std::size_t listenerHash = listenerHasher(listenerCallbacks.transactionCompletedListener);
+ std::size_t callbackIdsHash = callbackIdsHasher(listenerCallbacks.callbackIds);
+
+ return HashCombine(listenerHash, callbackIdsHash);
+ }
+};
+
class CallbackHandle : public RefBase {
public:
CallbackHandle(const sp<ITransactionCompletedListener>& transactionListener,
@@ -64,10 +87,12 @@
void run();
// Adds listener and callbackIds in case there are no SurfaceControls that are supposed
- // to be included in the callback. This functions should be call before attempting to add any
- // callback handles.
- status_t addCallback(const sp<ITransactionCompletedListener>& transactionListener,
- const std::vector<CallbackId>& callbackIds);
+ // to be included in the callback. This functions should be call before attempting to register
+ // any callback handles.
+ status_t startRegistration(const ListenerCallbacks& listenerCallbacks);
+ // Ends the registration. After this is called, no more CallbackHandles will be registered.
+ // It is safe to send a callback if the Transaction doesn't have any Pending callback handles.
+ status_t endRegistration(const ListenerCallbacks& listenerCallbacks);
// Informs the TransactionCompletedThread that there is a Transaction with a CallbackHandle
// that needs to be latched and presented this frame. This function should be called once the
@@ -76,11 +101,11 @@
// presented.
status_t registerPendingCallbackHandle(const sp<CallbackHandle>& handle);
// Notifies the TransactionCompletedThread that a pending CallbackHandle has been presented.
- status_t addPresentedCallbackHandles(const std::deque<sp<CallbackHandle>>& handles);
+ status_t finalizePendingCallbackHandles(const std::deque<sp<CallbackHandle>>& handles);
// Adds the Transaction CallbackHandle from a layer that does not need to be relatched and
// presented this frame.
- status_t addUnpresentedCallbackHandle(const sp<CallbackHandle>& handle);
+ status_t registerUnpresentedCallbackHandle(const sp<CallbackHandle>& handle);
void addPresentFence(const sp<Fence>& presentFence);
@@ -89,6 +114,9 @@
private:
void threadMain();
+ bool isRegisteringTransaction(const sp<ITransactionCompletedListener>& transactionListener,
+ const std::vector<CallbackId>& callbackIds) REQUIRES(mMutex);
+
status_t findTransactionStats(const sp<ITransactionCompletedListener>& listener,
const std::vector<CallbackId>& callbackIds,
TransactionStats** outTransactionStats) REQUIRES(mMutex);
@@ -106,13 +134,6 @@
};
sp<ThreadDeathRecipient> mDeathRecipient;
- struct ITransactionCompletedListenerHash {
- std::size_t operator()(const sp<ITransactionCompletedListener>& listener) const {
- return std::hash<IBinder*>{}((listener) ? IInterface::asBinder(listener).get()
- : nullptr);
- }
- };
-
// Protects the creation and destruction of mThread
std::mutex mThreadMutex;
@@ -121,11 +142,15 @@
std::mutex mMutex;
std::condition_variable_any mConditionVariable;
+ std::unordered_set<ListenerCallbacks, ListenerCallbacksHash> mRegisteringTransactions
+ GUARDED_BY(mMutex);
+
std::unordered_map<
sp<ITransactionCompletedListener>,
std::unordered_map<std::vector<CallbackId>, uint32_t /*count*/, CallbackIdsHash>,
ITransactionCompletedListenerHash>
mPendingTransactions GUARDED_BY(mMutex);
+
std::unordered_map<sp<ITransactionCompletedListener>, std::deque<TransactionStats>,
ITransactionCompletedListenerHash>
mCompletedTransactions GUARDED_BY(mMutex);