Skip to content

Commit fc36472

Browse files
committed
InputDispatcher: Fix crash when there is an ANR after window removal
This addresses a bug in the change: I47744cbd677cc74e26a102c50a2c11c68bc8aa89 InputDispatcher has an invariant that it can only send events to a connection if it has a window. We did not check if the channel receiving an ANR had a window before attempting to synthesize cancellations for it. Bug: 324330557 Bug: 210460522 Test: atest inputflinger_tests Change-Id: I5f3013fe93c0f4d1bb0f58e7b2a241cffe5c5bf2
1 parent 203b22a commit fc36472

2 files changed

Lines changed: 76 additions & 13 deletions

File tree

services/inputflinger/dispatcher/InputDispatcher.cpp

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2089,13 +2089,23 @@ void InputDispatcher::cancelEventsForAnrLocked(const std::shared_ptr<Connection>
20892089
// pile up.
20902090
ALOGW("Canceling events for %s because it is unresponsive",
20912091
connection->getInputChannelName().c_str());
2092-
if (connection->status == Connection::Status::NORMAL) {
2093-
CancelationOptions options(CancelationOptions::Mode::CANCEL_ALL_EVENTS,
2094-
"application not responding");
2095-
synthesizeCancelationEventsForConnectionLocked(connection, options,
2096-
getWindowHandleLocked(
2097-
connection->getToken()));
2092+
if (connection->status != Connection::Status::NORMAL) {
2093+
return;
2094+
}
2095+
CancelationOptions options(CancelationOptions::Mode::CANCEL_ALL_EVENTS,
2096+
"application not responding");
2097+
2098+
sp<WindowInfoHandle> windowHandle;
2099+
if (!connection->monitor) {
2100+
windowHandle = getWindowHandleLocked(connection->getToken());
2101+
if (windowHandle == nullptr) {
2102+
// The window that is receiving this ANR was removed, so there is no need to generate
2103+
// cancellations, because the cancellations would have already been generated when
2104+
// the window was removed.
2105+
return;
2106+
}
20982107
}
2108+
synthesizeCancelationEventsForConnectionLocked(connection, options, windowHandle);
20992109
}
21002110

21012111
void InputDispatcher::resetNoFocusedWindowTimeoutLocked() {

services/inputflinger/tests/InputDispatcher_test.cpp

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ static KeyEvent getTestKeyEvent() {
155155
class FakeInputDispatcherPolicy : public InputDispatcherPolicyInterface {
156156
struct AnrResult {
157157
sp<IBinder> token{};
158-
gui::Pid pid{gui::Pid::INVALID};
158+
std::optional<gui::Pid> pid{};
159159
};
160160
/* Stores data about a user-activity-poke event from the dispatcher. */
161161
struct UserActivityPokeEvent {
@@ -260,7 +260,7 @@ class FakeInputDispatcherPolicy : public InputDispatcherPolicyInterface {
260260

261261
void assertNotifyWindowUnresponsiveWasCalled(std::chrono::nanoseconds timeout,
262262
const sp<IBinder>& expectedToken,
263-
gui::Pid expectedPid) {
263+
std::optional<gui::Pid> expectedPid) {
264264
std::unique_lock lock(mLock);
265265
android::base::ScopedLockAssertion assumeLocked(mLock);
266266
AnrResult result;
@@ -280,7 +280,7 @@ class FakeInputDispatcherPolicy : public InputDispatcherPolicyInterface {
280280
}
281281

282282
void assertNotifyWindowResponsiveWasCalled(const sp<IBinder>& expectedToken,
283-
gui::Pid expectedPid) {
283+
std::optional<gui::Pid> expectedPid) {
284284
std::unique_lock lock(mLock);
285285
android::base::ScopedLockAssertion assumeLocked(mLock);
286286
AnrResult result;
@@ -524,16 +524,14 @@ class FakeInputDispatcherPolicy : public InputDispatcherPolicyInterface {
524524
void notifyWindowUnresponsive(const sp<IBinder>& connectionToken, std::optional<gui::Pid> pid,
525525
const std::string&) override {
526526
std::scoped_lock lock(mLock);
527-
ASSERT_TRUE(pid.has_value());
528-
mAnrWindows.push({connectionToken, *pid});
527+
mAnrWindows.push({connectionToken, pid});
529528
mNotifyAnr.notify_all();
530529
}
531530

532531
void notifyWindowResponsive(const sp<IBinder>& connectionToken,
533532
std::optional<gui::Pid> pid) override {
534533
std::scoped_lock lock(mLock);
535-
ASSERT_TRUE(pid.has_value());
536-
mResponsiveWindows.push({connectionToken, *pid});
534+
mResponsiveWindows.push({connectionToken, pid});
537535
mNotifyAnr.notify_all();
538536
}
539537

@@ -9059,6 +9057,61 @@ TEST_F(InputDispatcherSingleWindowAnr, TwoGesturesWithAnr) {
90599057
mWindow->consumeMotionEvent(WithMotionAction(ACTION_DOWN));
90609058
}
90619059

9060+
// Send an event to the app and have the app not respond right away. Then remove the app window.
9061+
// When the window is removed, the dispatcher will cancel the events for that window.
9062+
// So InputDispatcher will enqueue ACTION_CANCEL event as well.
9063+
TEST_F(InputDispatcherSingleWindowAnr, AnrAfterWindowRemoval) {
9064+
mDispatcher->notifyMotion(generateMotionArgs(AMOTION_EVENT_ACTION_DOWN,
9065+
AINPUT_SOURCE_TOUCHSCREEN, ADISPLAY_ID_DEFAULT,
9066+
{WINDOW_LOCATION}));
9067+
9068+
const auto [sequenceNum, _] = mWindow->receiveEvent(); // ACTION_DOWN
9069+
ASSERT_TRUE(sequenceNum);
9070+
9071+
// Remove the window, but the input channel should remain alive.
9072+
mDispatcher->onWindowInfosChanged({{}, {}, 0, 0});
9073+
9074+
const std::chrono::duration timeout = mWindow->getDispatchingTimeout(DISPATCHING_TIMEOUT);
9075+
// Since the window was removed, Dispatcher does not know the PID associated with the window
9076+
// anymore, so the policy is notified without the PID.
9077+
mFakePolicy->assertNotifyWindowUnresponsiveWasCalled(timeout, mWindow->getToken(),
9078+
/*pid=*/std::nullopt);
9079+
9080+
mWindow->finishEvent(*sequenceNum);
9081+
// The cancellation was generated when the window was removed, along with the focus event.
9082+
mWindow->consumeMotionEvent(
9083+
AllOf(WithMotionAction(ACTION_CANCEL), WithDisplayId(ADISPLAY_ID_DEFAULT)));
9084+
mWindow->consumeFocusEvent(false);
9085+
ASSERT_TRUE(mDispatcher->waitForIdle());
9086+
mFakePolicy->assertNotifyWindowResponsiveWasCalled(mWindow->getToken(), /*pid=*/std::nullopt);
9087+
}
9088+
9089+
// Send an event to the app and have the app not respond right away. Wait for the policy to be
9090+
// notified of the unresponsive window, then remove the app window.
9091+
TEST_F(InputDispatcherSingleWindowAnr, AnrFollowedByWindowRemoval) {
9092+
mDispatcher->notifyMotion(generateMotionArgs(AMOTION_EVENT_ACTION_DOWN,
9093+
AINPUT_SOURCE_TOUCHSCREEN, ADISPLAY_ID_DEFAULT,
9094+
{WINDOW_LOCATION}));
9095+
9096+
const auto [sequenceNum, _] = mWindow->receiveEvent(); // ACTION_DOWN
9097+
ASSERT_TRUE(sequenceNum);
9098+
const std::chrono::duration timeout = mWindow->getDispatchingTimeout(DISPATCHING_TIMEOUT);
9099+
mFakePolicy->assertNotifyWindowUnresponsiveWasCalled(timeout, mWindow);
9100+
9101+
// Remove the window, but the input channel should remain alive.
9102+
mDispatcher->onWindowInfosChanged({{}, {}, 0, 0});
9103+
9104+
mWindow->finishEvent(*sequenceNum);
9105+
// The cancellation was generated during the ANR, and the window lost focus when it was removed.
9106+
mWindow->consumeMotionEvent(
9107+
AllOf(WithMotionAction(ACTION_CANCEL), WithDisplayId(ADISPLAY_ID_DEFAULT)));
9108+
mWindow->consumeFocusEvent(false);
9109+
ASSERT_TRUE(mDispatcher->waitForIdle());
9110+
// Since the window was removed, Dispatcher does not know the PID associated with the window
9111+
// becoming responsive, so the policy is notified without the PID.
9112+
mFakePolicy->assertNotifyWindowResponsiveWasCalled(mWindow->getToken(), /*pid=*/std::nullopt);
9113+
}
9114+
90629115
class InputDispatcherMultiWindowAnr : public InputDispatcherTest {
90639116
virtual void SetUp() override {
90649117
InputDispatcherTest::SetUp();

0 commit comments

Comments
 (0)