Store connections by token instead of by fd
The connections are currently stored by fd. If a connection is removed
via 'removeInputChannel', it is possible to re-create the same
connection and have it keyed by the same fd. When this happens, a race
condition may occur where a socket hangup on this fd would cause the
removal of a newly registered connection.
In this refactor, the connections are no longer stored by fd. The looper
interface for adding fds has two versions:
1) the old one that we are currently using, which is marked as 'do not
use'
2) the new one where a callback object is provided instead.
In this CL, we switch to the new version of the callback.
There is now also no need to store the inputchannels in a separate
structure, because we can use the connections collection that's now
keyed by token to find them.
In a future refactor, we should switch to using 'unique_ptr' for the
inputchannels. Most of the time when we are looking for an input
channel, we are actually interested in finding the corresponding
connection.
If we switch Connection to shared_ptr, we can also look into switching
LooperEventCallback to store a weak pointer to a connection instead of
storing the connection token. This should speed up the handling of
events, by avoiding a map lookup.
Test: ./reinitinput.sh. Observe that it doesnt finish after this patch
Test: atest inputflinger_tests
Bug: 182478748
Change-Id: I601f765eebfadcaeff3661a10a10c4a4f0477389
diff --git a/services/inputflinger/dispatcher/InputDispatcher.h b/services/inputflinger/dispatcher/InputDispatcher.h
index 7ab4fd7..7ba03e8 100644
--- a/services/inputflinger/dispatcher/InputDispatcher.h
+++ b/services/inputflinger/dispatcher/InputDispatcher.h
@@ -211,9 +211,6 @@
bool addPortalWindows = false,
bool ignoreDragWindow = false) REQUIRES(mLock);
- // All registered connections mapped by channel file descriptor.
- std::unordered_map<int, sp<Connection>> mConnectionsByFd GUARDED_BY(mLock);
-
sp<Connection> getConnectionLocked(const sp<IBinder>& inputConnectionToken) const
REQUIRES(mLock);
@@ -225,8 +222,10 @@
struct StrongPointerHash {
std::size_t operator()(const sp<T>& b) const { return std::hash<T*>{}(b.get()); }
};
- std::unordered_map<sp<IBinder>, std::shared_ptr<InputChannel>, StrongPointerHash<IBinder>>
- mInputChannelsByToken GUARDED_BY(mLock);
+
+ // All registered connections mapped by input channel token.
+ std::unordered_map<sp<IBinder>, sp<Connection>, StrongPointerHash<IBinder>> mConnectionsByToken
+ GUARDED_BY(mLock);
// Finds the display ID of the gesture monitor identified by the provided token.
std::optional<int32_t> findGestureMonitorDisplayByTokenLocked(const sp<IBinder>& token)
@@ -544,7 +543,7 @@
bool notify) REQUIRES(mLock);
void drainDispatchQueue(std::deque<DispatchEntry*>& queue);
void releaseDispatchEntry(DispatchEntry* dispatchEntry);
- static int handleReceiveCallback(int fd, int events, void* data);
+ int handleReceiveCallback(int events, sp<IBinder> connectionToken);
// The action sent should only be of type AMOTION_EVENT_*
void dispatchPointerDownOutsideFocus(uint32_t source, int32_t action,
const sp<IBinder>& newToken) REQUIRES(mLock);