OSDN Git Service

Fix another deadlock between IMMS and TSMS
authorYohei Yukawa <yukawa@google.com>
Tue, 20 Sep 2016 13:43:03 +0000 (06:43 -0700)
committerYohei Yukawa <yukawa@google.com>
Tue, 20 Sep 2016 13:43:03 +0000 (06:43 -0700)
Bug 31247871 and Bug 31273203 are the same in terms of that both can be
triggered by calling TSM##getCurrentSpellCheckerSubtype() but different
in terms of what lock objects are involved.

To summarize

 Bug 31273203: between IMMS#mMethodMap and IMM#H
  A. OnClickListener.onClick() running in the IMMS locks IMMS#mMethodMap
     then does some View operations, which can be blocked until
     IMM#H is unlocked (e.g. IMM#onViewDetachedFromWindow()).
  B. TSMS#getCurrentSpellCheckerSubtype() internally calls
     IMM#getCurrentInputMethodSubtype(), which locks IMM#H then can be
     blocked until IMMS#mMethodMap is unlocked.
  The tricky point here is that IMMS and TSMS are running in the same
  process hence IMM#H are actually shared between them.

 Bug 31247871: between IMMS#mMethodMap and TSMS#mSpellCheckerMap
  C. IMMS locks IMMS#mMethodMap then calls
     InputMethodUtils#setNonSelectedSystemImesDisabledUntilUsed(), which
     can be blocked until TSMS#mSpellCheckerMap is unlocked.
  D. TSMS#getCurrentSpellCheckerSubtype() locks TSMS#mSpellCheckerMap
     then may call IMM#getCurrentInputMethodSubtype(), which can be
     blocked until IMMS#mMethodMap is released.

This CL aims to remove the layered lock in D to close Bug 31247871,
while the previous CL [1] took care of B to close Bug 31273203.

Note that A and C are still concerning and should also be taken care of
as a part of Bug 31273203.

 [1]: I20cf2c20f49b1b02c0f7a18257b49d4bcc081b5d
      fa1886feea55785f413f5efcd86bccca92f26759

Bug: 31247871
Bug: 31273203
Change-Id: I26479e7aa306e0df91d9911891d5625dd5f99678

services/core/java/com/android/server/TextServicesManagerService.java

index 4b0d4be..4f02a23 100644 (file)
@@ -459,71 +459,75 @@ public class TextServicesManagerService extends ITextServicesManager.Stub {
         if (!calledFromValidUser()) {
             return null;
         }
+        final int subtypeHashCode;
+        final SpellCheckerInfo sci;
+        final Locale systemLocale;
         synchronized (mSpellCheckerMap) {
-            final int subtypeHashCode =
+            subtypeHashCode =
                     mSettings.getSelectedSpellCheckerSubtype(SpellCheckerSubtype.SUBTYPE_ID_NONE);
             if (DBG) {
                 Slog.w(TAG, "getCurrentSpellCheckerSubtype: " + subtypeHashCode);
             }
-            final SpellCheckerInfo sci = getCurrentSpellChecker(null);
-            if (sci == null || sci.getSubtypeCount() == 0) {
-                if (DBG) {
-                    Slog.w(TAG, "Subtype not found.");
+            sci = getCurrentSpellChecker(null);
+            systemLocale = mContext.getResources().getConfiguration().locale;
+        }
+        if (sci == null || sci.getSubtypeCount() == 0) {
+            if (DBG) {
+                Slog.w(TAG, "Subtype not found.");
+            }
+            return null;
+        }
+        if (subtypeHashCode == SpellCheckerSubtype.SUBTYPE_ID_NONE
+                && !allowImplicitlySelectedSubtype) {
+            return null;
+        }
+        String candidateLocale = null;
+        if (subtypeHashCode == 0) {
+            // Spell checker language settings == "auto"
+            final InputMethodManager imm = mContext.getSystemService(InputMethodManager.class);
+            if (imm != null) {
+                final InputMethodSubtype currentInputMethodSubtype =
+                        imm.getCurrentInputMethodSubtype();
+                if (currentInputMethodSubtype != null) {
+                    final String localeString = currentInputMethodSubtype.getLocale();
+                    if (!TextUtils.isEmpty(localeString)) {
+                        // 1. Use keyboard locale if available in the spell checker
+                        candidateLocale = localeString;
+                    }
                 }
-                return null;
             }
-            if (subtypeHashCode == SpellCheckerSubtype.SUBTYPE_ID_NONE
-                    && !allowImplicitlySelectedSubtype) {
-                return null;
+            if (candidateLocale == null) {
+                // 2. Use System locale if available in the spell checker
+                candidateLocale = systemLocale.toString();
             }
-            String candidateLocale = null;
+        }
+        SpellCheckerSubtype candidate = null;
+        for (int i = 0; i < sci.getSubtypeCount(); ++i) {
+            final SpellCheckerSubtype scs = sci.getSubtypeAt(i);
             if (subtypeHashCode == 0) {
-                // Spell checker language settings == "auto"
-                final InputMethodManager imm = mContext.getSystemService(InputMethodManager.class);
-                if (imm != null) {
-                    final InputMethodSubtype currentInputMethodSubtype =
-                            imm.getCurrentInputMethodSubtype();
-                    if (currentInputMethodSubtype != null) {
-                        final String localeString = currentInputMethodSubtype.getLocale();
-                        if (!TextUtils.isEmpty(localeString)) {
-                            // 1. Use keyboard locale if available in the spell checker
-                            candidateLocale = localeString;
-                        }
+                final String scsLocale = scs.getLocale();
+                if (candidateLocale.equals(scsLocale)) {
+                    return scs;
+                } else if (candidate == null) {
+                    if (candidateLocale.length() >= 2 && scsLocale.length() >= 2
+                            && candidateLocale.startsWith(scsLocale)) {
+                        // Fall back to the applicable language
+                        candidate = scs;
                     }
                 }
-                if (candidateLocale == null) {
-                    // 2. Use System locale if available in the spell checker
-                    candidateLocale = mContext.getResources().getConfiguration().locale.toString();
-                }
-            }
-            SpellCheckerSubtype candidate = null;
-            for (int i = 0; i < sci.getSubtypeCount(); ++i) {
-                final SpellCheckerSubtype scs = sci.getSubtypeAt(i);
-                if (subtypeHashCode == 0) {
-                    final String scsLocale = scs.getLocale();
-                    if (candidateLocale.equals(scsLocale)) {
-                        return scs;
-                    } else if (candidate == null) {
-                        if (candidateLocale.length() >= 2 && scsLocale.length() >= 2
-                                && candidateLocale.startsWith(scsLocale)) {
-                            // Fall back to the applicable language
-                            candidate = scs;
-                        }
-                    }
-                } else if (scs.hashCode() == subtypeHashCode) {
-                    if (DBG) {
-                        Slog.w(TAG, "Return subtype " + scs.hashCode() + ", input= " + locale
-                                + ", " + scs.getLocale());
-                    }
-                    // 3. Use the user specified spell check language
-                    return scs;
+            } else if (scs.hashCode() == subtypeHashCode) {
+                if (DBG) {
+                    Slog.w(TAG, "Return subtype " + scs.hashCode() + ", input= " + locale
+                            + ", " + scs.getLocale());
                 }
+                // 3. Use the user specified spell check language
+                return scs;
             }
-            // 4. Fall back to the applicable language and return it if not null
-            // 5. Simply just return it even if it's null which means we could find no suitable
-            // spell check languages
-            return candidate;
         }
+        // 4. Fall back to the applicable language and return it if not null
+        // 5. Simply just return it even if it's null which means we could find no suitable
+        // spell check languages
+        return candidate;
     }
 
     @Override