Notify policy about broken input channels
The InputDispatcherPolicyInterface contains a callback for
'notifyInputChannelBroken'. Looking at the implementation of the policy
for this callback, inside InputManagerCallback.java, the intent of this
callback is to call windowState.removeIfPossible(). In other words, try
to remove the window when the input channel is broken.
In the input code, this could be triggered in 3 different ways:
1) pipe is full (WOULD_BLOCK), but the wait queue is empty
2) can't write to socket, but the error is other than 'WOULD_BLOCK'
3) 'removeInputChannelLocked' is called
The first two cases don't really happen in practice. The last case can
happen when the app closes the socket, but the window still remains on
the screen (maybe temporarily).
It turns out, before this patch, the policy was never getting notified
for the case 3) either. That means, this API has not been called in a
very long time (> 7 years from brief look through the code history).
It was never getting called because the policy callback is skipped if
the input connection is marked as 'zombie'. However, in
'removeInputChannelLocked', the connection is marked as 'zombie' as soon
as the notifyInputChannelBroken command is queued. So in practice, the
connection is already zombie before the policy notification can happen.
In this CL, we remove the check for the connection being zombie. We will
always notify, since once an fd is closed, there's no way to recover
from that.
If this CL causes breakages because this code path is now going to be
triggered, we should either fix the WM implementation of this callback,
or we should just remove this policy callback altogether.
Bug: 206861133
Test: atest inputflinger_tests
Change-Id: Iaaf5e9c3a2681d7afa444913bd6f2686c413e7df
diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp
index 515a01e..c5d9e66 100644
--- a/services/inputflinger/tests/InputDispatcher_test.cpp
+++ b/services/inputflinger/tests/InputDispatcher_test.cpp
@@ -219,21 +219,21 @@
template <class T>
T getAnrTokenLockedInterruptible(std::chrono::nanoseconds timeout, std::queue<T>& storage,
std::unique_lock<std::mutex>& lock) REQUIRES(mLock) {
- const std::chrono::time_point start = std::chrono::steady_clock::now();
- std::chrono::duration timeToWait = timeout + 100ms; // provide some slack
-
// If there is an ANR, Dispatcher won't be idle because there are still events
// in the waitQueue that we need to check on. So we can't wait for dispatcher to be idle
// before checking if ANR was called.
// Since dispatcher is not guaranteed to call notifyNoFocusedWindowAnr right away, we need
// to provide it some time to act. 100ms seems reasonable.
- mNotifyAnr.wait_for(lock, timeToWait,
- [&storage]() REQUIRES(mLock) { return !storage.empty(); });
- const std::chrono::duration waited = std::chrono::steady_clock::now() - start;
- if (storage.empty()) {
+ std::chrono::duration timeToWait = timeout + 100ms; // provide some slack
+ const std::chrono::time_point start = std::chrono::steady_clock::now();
+ std::optional<T> token =
+ getItemFromStorageLockedInterruptible(timeToWait, storage, lock, mNotifyAnr);
+ if (!token.has_value()) {
ADD_FAILURE() << "Did not receive the ANR callback";
return {};
}
+
+ const std::chrono::duration waited = std::chrono::steady_clock::now() - start;
// Ensure that the ANR didn't get raised too early. We can't be too strict here because
// the dispatcher started counting before this function was called
if (std::chrono::abs(timeout - waited) > 100ms) {
@@ -243,9 +243,24 @@
<< std::chrono::duration_cast<std::chrono::milliseconds>(waited).count()
<< "ms instead";
}
- T token = storage.front();
+ return *token;
+ }
+
+ template <class T>
+ std::optional<T> getItemFromStorageLockedInterruptible(std::chrono::nanoseconds timeout,
+ std::queue<T>& storage,
+ std::unique_lock<std::mutex>& lock,
+ std::condition_variable& condition)
+ REQUIRES(mLock) {
+ condition.wait_for(lock, timeout,
+ [&storage]() REQUIRES(mLock) { return !storage.empty(); });
+ if (storage.empty()) {
+ ADD_FAILURE() << "Did not receive the expected callback";
+ return std::nullopt;
+ }
+ T item = storage.front();
storage.pop();
- return token;
+ return std::make_optional(item);
}
void assertNotifyAnrWasNotCalled() {
@@ -303,6 +318,16 @@
mNotifyDropWindowWasCalled = false;
}
+ void assertNotifyInputChannelBrokenWasCalled(const sp<IBinder>& token) {
+ std::unique_lock lock(mLock);
+ base::ScopedLockAssertion assumeLocked(mLock);
+ std::optional<sp<IBinder>> receivedToken =
+ getItemFromStorageLockedInterruptible(100ms, mBrokenInputChannels, lock,
+ mNotifyInputChannelBroken);
+ ASSERT_TRUE(receivedToken.has_value());
+ ASSERT_EQ(token, *receivedToken);
+ }
+
private:
std::mutex mLock;
std::unique_ptr<InputEvent> mFilteredEvent GUARDED_BY(mLock);
@@ -321,6 +346,8 @@
std::queue<int32_t> mAnrMonitorPids GUARDED_BY(mLock);
std::queue<int32_t> mResponsiveMonitorPids GUARDED_BY(mLock);
std::condition_variable mNotifyAnr;
+ std::queue<sp<IBinder>> mBrokenInputChannels GUARDED_BY(mLock);
+ std::condition_variable mNotifyInputChannelBroken;
sp<IBinder> mDropTargetWindowToken GUARDED_BY(mLock);
bool mNotifyDropWindowWasCalled GUARDED_BY(mLock) = false;
@@ -361,7 +388,11 @@
mNotifyAnr.notify_all();
}
- void notifyInputChannelBroken(const sp<IBinder>&) override {}
+ void notifyInputChannelBroken(const sp<IBinder>& connectionToken) override {
+ std::scoped_lock lock(mLock);
+ mBrokenInputChannels.push(connectionToken);
+ mNotifyInputChannelBroken.notify_all();
+ }
void notifyFocusChanged(const sp<IBinder>&, const sp<IBinder>&) override {}
@@ -1175,6 +1206,8 @@
mInfo.ownerUid = ownerUid;
}
+ void destroyReceiver() { mInputReceiver = nullptr; }
+
private:
const std::string mName;
std::unique_ptr<FakeInputReceiver> mInputReceiver;
@@ -1438,6 +1471,23 @@
return NotifyPointerCaptureChangedArgs(/* id */ 0, systemTime(SYSTEM_TIME_MONOTONIC), request);
}
+/**
+ * When a window unexpectedly disposes of its input channel, policy should be notified about the
+ * broken channel.
+ */
+TEST_F(InputDispatcherTest, WhenInputChannelBreaks_PolicyIsNotified) {
+ std::shared_ptr<FakeApplicationHandle> application = std::make_shared<FakeApplicationHandle>();
+ sp<FakeWindowHandle> window =
+ new FakeWindowHandle(application, mDispatcher, "Window that breaks its input channel",
+ ADISPLAY_ID_DEFAULT);
+
+ mDispatcher->setInputWindows({{ADISPLAY_ID_DEFAULT, {window}}});
+
+ // Window closes its channel, but the window remains.
+ window->destroyReceiver();
+ mFakePolicy->assertNotifyInputChannelBrokenWasCalled(window->getInfo()->token);
+}
+
TEST_F(InputDispatcherTest, SetInputWindow_SingleWindowTouch) {
std::shared_ptr<FakeApplicationHandle> application = std::make_shared<FakeApplicationHandle>();
sp<FakeWindowHandle> window =