Switch from ITransactionCompletedListener to IBinder keys
IBinder remains consistent across multiple transactions when passed
cross-process to SF. However, interface_cast'ing it to an
ITransactionCompletedListener results in different underlying objects.
Switch to using IBinder as the keys for listener callbacks.
Bug: 139731321
Test: build, boot, SurfaceFlinger_test
Change-Id: I9bea76664087020eb43ceec258b5ecede0faaea5
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index e199ad5..5e263b2 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -29,6 +29,7 @@
#include <gui/FrameTimestamps.h>
#include <gui/ISurfaceComposer.h>
#include <gui/ISurfaceComposerClient.h>
+#include <gui/ITransactionCompletedListener.h>
#include <gui/LayerState.h>
#include <gui/OccupancyTracker.h>
#include <hardware/hwcomposer_defs.h>
@@ -1025,11 +1026,6 @@
uint32_t mTexturePoolSize = 0;
std::vector<uint32_t> mTexturePool;
- struct IBinderHash {
- std::size_t operator()(const sp<IBinder>& strongPointer) const {
- return std::hash<IBinder*>{}(strongPointer.get());
- }
- };
struct TransactionState {
TransactionState(const Vector<ComposerState>& composerStates,
const Vector<DisplayState>& displayStates, uint32_t transactionFlags,
@@ -1054,7 +1050,7 @@
const int64_t postTime;
bool privileged;
};
- std::unordered_map<sp<IBinder>, std::queue<TransactionState>, IBinderHash> mTransactionQueues;
+ std::unordered_map<sp<IBinder>, std::queue<TransactionState>, IListenerHash> mTransactionQueues;
/* ------------------------------------------------------------------------
* Feature prototyping
diff --git a/services/surfaceflinger/TransactionCompletedThread.cpp b/services/surfaceflinger/TransactionCompletedThread.cpp
index c519f8d..1324f20 100644
--- a/services/surfaceflinger/TransactionCompletedThread.cpp
+++ b/services/surfaceflinger/TransactionCompletedThread.cpp
@@ -24,7 +24,6 @@
#include <cinttypes>
#include <binder/IInterface.h>
-#include <gui/ITransactionCompletedListener.h>
#include <utils/RefBase.h>
namespace android {
@@ -58,7 +57,7 @@
{
std::lock_guard lock(mMutex);
for (const auto& [listener, transactionStats] : mCompletedTransactions) {
- IInterface::asBinder(listener)->unlinkToDeath(mDeathRecipient);
+ listener->unlinkToDeath(mDeathRecipient);
}
}
}
@@ -85,7 +84,7 @@
auto& [listener, callbackIds] = listenerCallbacks;
if (mCompletedTransactions.count(listener) == 0) {
- status_t err = IInterface::asBinder(listener)->linkToDeath(mDeathRecipient);
+ status_t err = listener->linkToDeath(mDeathRecipient);
if (err != NO_ERROR) {
ALOGE("cannot add callback because linkToDeath failed, err: %d", err);
return err;
@@ -119,8 +118,7 @@
}
bool TransactionCompletedThread::isRegisteringTransaction(
- const sp<ITransactionCompletedListener>& transactionListener,
- const std::vector<CallbackId>& callbackIds) {
+ const sp<IBinder>& transactionListener, const std::vector<CallbackId>& callbackIds) {
ListenerCallbacks listenerCallbacks(transactionListener, callbackIds);
auto itr = mRegisteringTransactions.find(listenerCallbacks);
@@ -201,8 +199,8 @@
}
status_t TransactionCompletedThread::findTransactionStats(
- const sp<ITransactionCompletedListener>& listener,
- const std::vector<CallbackId>& callbackIds, TransactionStats** outTransactionStats) {
+ const sp<IBinder>& listener, const std::vector<CallbackId>& callbackIds,
+ TransactionStats** outTransactionStats) {
auto& transactionStatsDeque = mCompletedTransactions[listener];
// Search back to front because the most recent transactions are at the back of the deque
@@ -300,10 +298,16 @@
// If the listener has completed transactions
if (!listenerStats.transactionStats.empty()) {
// If the listener is still alive
- if (IInterface::asBinder(listener)->isBinderAlive()) {
- // Send callback
- listenerStats.listener->onTransactionCompleted(listenerStats);
- IInterface::asBinder(listener)->unlinkToDeath(mDeathRecipient);
+ if (listener->isBinderAlive()) {
+ // Send callback. The listener stored in listenerStats
+ // comes from the cross-process setTransactionState call to
+ // SF. This MUST be an ITransactionCompletedListener. We
+ // keep it as an IBinder due to consistency reasons: if we
+ // interface_cast at the IPC boundary when reading a Parcel,
+ // we get pointers that compare unequal in the SF process.
+ interface_cast<ITransactionCompletedListener>(listenerStats.listener)
+ ->onTransactionCompleted(listenerStats);
+ listener->unlinkToDeath(mDeathRecipient);
}
completedTransactionsItr = mCompletedTransactions.erase(completedTransactionsItr);
} else {
@@ -335,7 +339,7 @@
// -----------------------------------------------------------------------
-CallbackHandle::CallbackHandle(const sp<ITransactionCompletedListener>& transactionListener,
+CallbackHandle::CallbackHandle(const sp<IBinder>& transactionListener,
const std::vector<CallbackId>& ids, const sp<IBinder>& sc)
: listener(transactionListener), callbackIds(ids), surfaceControl(sc) {}
diff --git a/services/surfaceflinger/TransactionCompletedThread.h b/services/surfaceflinger/TransactionCompletedThread.h
index e255e50..a85ad1e 100644
--- a/services/surfaceflinger/TransactionCompletedThread.h
+++ b/services/surfaceflinger/TransactionCompletedThread.h
@@ -31,46 +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
- // empty we can still hash 0.
- // 2) CallbackId vectors for the same listener either are identical or contain none of the
- // same members. It is sufficient to just check the first CallbackId in the vectors. If
- // they match, they are the same. If they do not match, they are not the same.
- std::size_t operator()(const std::vector<CallbackId>& callbackIds) const {
- return std::hash<CallbackId>{}((callbackIds.empty()) ? 0 : callbackIds.front());
- }
-};
-
-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,
- const std::vector<CallbackId>& ids, const sp<IBinder>& sc);
+ CallbackHandle(const sp<IBinder>& transactionListener, const std::vector<CallbackId>& ids,
+ const sp<IBinder>& sc);
- sp<ITransactionCompletedListener> listener;
+ sp<IBinder> listener;
std::vector<CallbackId> callbackIds;
wp<IBinder> surfaceControl;
@@ -114,10 +80,10 @@
private:
void threadMain();
- bool isRegisteringTransaction(const sp<ITransactionCompletedListener>& transactionListener,
+ bool isRegisteringTransaction(const sp<IBinder>& transactionListener,
const std::vector<CallbackId>& callbackIds) REQUIRES(mMutex);
- status_t findTransactionStats(const sp<ITransactionCompletedListener>& listener,
+ status_t findTransactionStats(const sp<IBinder>& listener,
const std::vector<CallbackId>& callbackIds,
TransactionStats** outTransactionStats) REQUIRES(mMutex);
@@ -146,13 +112,12 @@
GUARDED_BY(mMutex);
std::unordered_map<
- sp<ITransactionCompletedListener>,
+ sp<IBinder>,
std::unordered_map<std::vector<CallbackId>, uint32_t /*count*/, CallbackIdsHash>,
- ITransactionCompletedListenerHash>
+ IListenerHash>
mPendingTransactions GUARDED_BY(mMutex);
- std::unordered_map<sp<ITransactionCompletedListener>, std::deque<TransactionStats>,
- ITransactionCompletedListenerHash>
+ std::unordered_map<sp<IBinder>, std::deque<TransactionStats>, IListenerHash>
mCompletedTransactions GUARDED_BY(mMutex);
bool mRunning GUARDED_BY(mMutex) = false;