From e3e31a880169a5d685ea4bf7efae81430a62b4fe Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Tue, 20 Sep 2016 06:43:03 -0700 Subject: [PATCH] Fix another deadlock between IMMS and TSMS 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 --- .../server/TextServicesManagerService.java | 122 +++++++++--------- 1 file changed, 63 insertions(+), 59 deletions(-) diff --git a/services/core/java/com/android/server/TextServicesManagerService.java b/services/core/java/com/android/server/TextServicesManagerService.java index 4b0d4be11b148..4f02a23d1e764 100644 --- a/services/core/java/com/android/server/TextServicesManagerService.java +++ b/services/core/java/com/android/server/TextServicesManagerService.java @@ -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."); - } - 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; - } - } - } - 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; - } - } - // 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; + 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; + } + } + } + if (candidateLocale == null) { + // 2. Use System locale if available in the spell checker + candidateLocale = systemLocale.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; + } + } + // 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