Fix system restart due to race in TSMS
This is a follow up CL to my previous CL [1], which aimed to get rid of
sync IPCs from TextServicesManagerService (TSMS) to
SpellCheckerService (SCS).
What was overlooked is that mSomeArrayList.forEach(action) throws
ConcurrentModificationException when the specified action adds or
removes items to/from mSomeArrayList. This can happen in my previous CL
when
1. An app requests a new spell checker service.
2. TSMS puts the request to mPendingSessionRequests and launchs the
current SCS by binding to it.
3. SCS#onCreate() succeeds.
4. TSMS receives onServiceConnected() callback.
4. SCS crashes.
5. TSMS tries to dispatch pending requests to the SCS as follows
mPendingSessionRequests.forEach(this::getISpellCheckerSessionLocked)
6. In getISpellCheckerSessionLocked() the IPC to the SCS fails because
the target SCS process is already dead. This triggeres removeAll(),
which clears mPendingSessionRequests.
7. ConcurrentModificationException is thrown, which will never be
caught and results in system restart.
Essentially this is a timing issue, but you can most likely reproduce
the issue by manually adding a certain delay in
SpellCheckerBindGroup#onServiceConnected() as follows.
public void onServiceConnectedLocked(ISpellCheckerService spellChecker) {
if (DBG) {
Slog.d(TAG, "onServiceConnected");
}
+ try {
+ Thread.sleep(100 /* msec */);
+ } catch (InterruptedException e) {
+ }
mSpellChecker = spellChecker;
mConnected = true;
// Dispatch pending getISpellCheckerSession requests.
mPendingSessionRequests.forEach(this::getISpellCheckerSessionLocked);
mPendingSessionRequests.clear();
}
Then you can also emulate the SCS crash as follows.
public class CrashingSpellCheckerService extends SpellCheckerService {
@Override
public void onCreate() {
super.onCreate();
new Thread(new Runnable() {
@Override
public void run() {
try {
Thread.sleep(10 /* msec */);
} catch (InterruptedException e) {
}
Process.killProcess(Process.myPid());
}
}).start();
}
...
}
This CL addresses the above issue by simply inlining
getISpellCheckerSessionLocked() to each call site to make it clear what
will happen in RemoteException is thrown.
[1]: I92e7aa40dc9ea14f67d355f0bfa15325b775d27b
7fa65eef203c4ed3ce00ddef96ccf311d3bfb58c
Test: Manually made sure that without this CL the system can restart
with the above technique.
Test: Manually made sure that AOSP spell checker service still works
as expected.
Fixes: 63452807
Change-Id: I6de5ac0507574f28a9859b50d081378112f1f7df
1 file changed