From 85d8f5c669699d28377547b3c4656c4001c32720 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Sun, 18 Apr 2021 07:50:14 +0000 Subject: [PATCH] Do not recreate ImeInputEventSender for the same channel InputEventSender is used to send events to an InputChannel. On the other side, InputEventReceiver receives the events and processes them. There's no good way to synchronize InputEventSender and InputEventReceiver today. If the InputEventSender object changes in the middle of interaction with the InputEventReceiver, it will confuse the receiver. Recently added strict checking of state in InputEventReceiver causes the other side to crash. In the IME case, there are dup'ed InputChannel being compared to each other by object. Since the objects are different, the equality check fails, even though they represent the same connection. This causes the InputEventReceiver to receive a sequence of events like this: incoming event seq = 1 incoming event seq = 2 incoming event seq = 3 incoming event seq = 1 incoming event seq = 2 ... If the InputEventReceiver is slow, it might only send finished signal after it processed first event with seq = 2, but the sender has already changed, and has already sent a new event with seq = 2. This would cause the receiver to overwrite the state of the second event with the first event, thus later causing a crash when it tries to ack the second event. The events received by the InputEventReceiver should all have unique seq. The pattern of re-creating InputEventSender breaks this contract. To fix this issue, we check whether the provided InputChannel represents the same connection. If the connection is the same, we do not re-create the InputEventSender, and just keep using the same object. This ensures that InputEventReceiver is always communicating with only one InputEventSender and the state remains consistent. In this CL, only a minimal change is made, and it is not rootcaused why the 'setInputChannelLocked' call is being made for the same connection. Bug: 183434055 Test: adb shell monkey 10000; run this test several times until it crashes. No more crashes observed with "Abort message: 'Could not find consume time for seq=2'" Test: use soft keyboard on device Change-Id: If8317941dd1c6d5db77f1239b9a9f45d49997df9 --- .../view/inputmethod/InputMethodManager.java | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/core/java/android/view/inputmethod/InputMethodManager.java b/core/java/android/view/inputmethod/InputMethodManager.java index 42985501d818c..7eb840047c9f5 100644 --- a/core/java/android/view/inputmethod/InputMethodManager.java +++ b/core/java/android/view/inputmethod/InputMethodManager.java @@ -1519,17 +1519,25 @@ public final class InputMethodManager { } void setInputChannelLocked(InputChannel channel) { - if (mCurChannel != channel) { - if (mCurSender != null) { - flushPendingEventsLocked(); - mCurSender.dispose(); - mCurSender = null; - } - if (mCurChannel != null) { - mCurChannel.dispose(); - } - mCurChannel = channel; + if (mCurChannel == channel) { + return; } + if (mCurChannel != null && channel != null + && mCurChannel.getToken() == channel.getToken()) { + // channel is a dupe of 'mCurChannel', because they have the same token, and represent + // the same connection. Ignore the incoming channel and keep using 'mCurChannel' to + // avoid confusing the InputEventReceiver. + return; + } + if (mCurSender != null) { + flushPendingEventsLocked(); + mCurSender.dispose(); + mCurSender = null; + } + if (mCurChannel != null) { + mCurChannel.dispose(); + } + mCurChannel = channel; } /**