From 73ac947f6ea3cf12f88f48f181e7917d705997eb Mon Sep 17 00:00:00 2001
From: Yohei Yukawa <yukawa@google.com>
Date: Tue, 1 May 2018 11:40:29 -0700
Subject: [PATCH] Fix a race in spell checker session handling

This is a follow up CL to my previous CL [1] that eliminated
synchronous IPCs from TextServicesManagerService (TSMS) to
SpellCheckerService (SCS).

One thing I missed was that even while TSMS is waiting for the
response from SCS pending sessions can be terminated from the client
application side (e.g. SpellCheckerSession#close() is explicitly
called or the client application process gets killed).

With this CL, SpellCheckerBindGroup#removeListener() can work even in
such situations.

 [1]: I92e7aa40dc9ea14f67d355f0bfa15325b775d27b
      7fa65eef203c4ed3ce00ddef96ccf311d3bfb58c

Fix: 79082580
Test: Manually done as follows.
  1. Create a custom spell checker service that can emulate the
     scenarios where following methods are blocked
     * SpellCheckerService#onCreate()
     * SpellCheckerService#createSession()
     * SpellCheckerService.Session#onGetSuggestions()
     * SpellCheckerService.Session#onGetSentenceSuggestionsMultiple()
  2. By using such a test spell checker service, make sure that TSMS
     cleans up ISpellCheckerSessionListener and
     ITextServicesSessionListener binder proxies when either
     SpellCheckerSession#close() is called or the target app process
     is killed.
Change-Id: Ie373095f20dd58273c764f1f732631bcfee758d7
---
 .../com/android/server/TextServicesManagerService.java    | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/services/core/java/com/android/server/TextServicesManagerService.java b/services/core/java/com/android/server/TextServicesManagerService.java
index 26a8cf74d245b..58d53403ed061 100644
--- a/services/core/java/com/android/server/TextServicesManagerService.java
+++ b/services/core/java/com/android/server/TextServicesManagerService.java
@@ -71,6 +71,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.function.Predicate;
 
 public class TextServicesManagerService extends ITextServicesManager.Stub {
     private static final String TAG = TextServicesManagerService.class.getSimpleName();
@@ -885,6 +886,11 @@ public class TextServicesManagerService extends ITextServicesManager.Stub {
             }
             synchronized (mLock) {
                 mListeners.unregister(listener);
+                final IBinder scListenerBinder = listener.asBinder();
+                final Predicate<SessionRequest> removeCondition =
+                        request -> request.mScListener.asBinder() == scListenerBinder;
+                mPendingSessionRequests.removeIf(removeCondition);
+                mOnGoingSessionRequests.removeIf(removeCondition);
                 cleanLocked();
             }
         }
@@ -934,6 +940,7 @@ public class TextServicesManagerService extends ITextServicesManager.Stub {
             if (mUnbindCalled) {
                 return;
             }
+            mListeners.register(request.mScListener);
             if (!mConnected) {
                 mPendingSessionRequests.add(request);
                 return;
@@ -959,7 +966,6 @@ public class TextServicesManagerService extends ITextServicesManager.Stub {
                 if (mOnGoingSessionRequests.remove(request)) {
                     try {
                         request.mTsListener.onServiceConnected(newSession);
-                        mListeners.register(request.mScListener);
                     } catch (RemoteException e) {
                         // Technically this can happen if the spell checker client app is already
                         // dead.  We can just forget about this request; the request is already