This class also takes care of a special case when system_server is also an IME client. In this + * scenario methods defined in {@link IInputMethodClient} are invoked as an direct (sync) invocation + * despite the fact that all of them are marked {@code oneway}. This can easily cause dead lock + * because {@link InputMethodManagerService} assumes that it's safe to make one-way IPCs while + * holding the lock. This assumption becomes invalid when {@link IInputMethodClient} is not a + * {@link android.os.BinderProxy}.
+ * + *To work around such a special scenario, this wrapper re-dispatch the method invocation into + * the given {@link Handler} thread if {@link IInputMethodClient} is not a proxy object. Be careful + * about its call ordering characteristics.
*/ final class IInputMethodClientInvoker { private static final String TAG = InputMethodManagerService.TAG; @@ -38,19 +50,25 @@ final class IInputMethodClientInvoker { @NonNull private final IInputMethodClient mTarget; private final boolean mIsProxy; + @Nullable + private final Handler mHandler; @AnyThread @Nullable - static IInputMethodClientInvoker create(@Nullable IInputMethodClient inputMethodClient) { + static IInputMethodClientInvoker create(@Nullable IInputMethodClient inputMethodClient, + @NonNull Handler handler) { if (inputMethodClient == null) { return null; } - return new IInputMethodClientInvoker(inputMethodClient); + final boolean isProxy = Binder.isProxy(inputMethodClient); + return new IInputMethodClientInvoker(inputMethodClient, isProxy, isProxy ? null : handler); } - private IInputMethodClientInvoker(@NonNull IInputMethodClient target) { + private IInputMethodClientInvoker(@NonNull IInputMethodClient target, + boolean isProxy, @Nullable Handler handler) { mTarget = target; - mIsProxy = Binder.isProxy(mTarget); + mIsProxy = isProxy; + mHandler = handler; } /** @@ -76,6 +94,15 @@ final class IInputMethodClientInvoker { @AnyThread void onBindMethod(@NonNull InputBindResult res) { + if (mIsProxy) { + onBindMethodInternal(res); + } else { + mHandler.post(() -> onBindMethodInternal(res)); + } + } + + @AnyThread + private void onBindMethodInternal(@NonNull InputBindResult res) { try { mTarget.onBindMethod(res); } catch (RemoteException e) { @@ -91,6 +118,15 @@ final class IInputMethodClientInvoker { @AnyThread void onBindAccessibilityService(@NonNull InputBindResult res, int id) { + if (mIsProxy) { + onBindAccessibilityServiceInternal(res, id); + } else { + mHandler.post(() -> onBindAccessibilityServiceInternal(res, id)); + } + } + + @AnyThread + private void onBindAccessibilityServiceInternal(@NonNull InputBindResult res, int id) { try { mTarget.onBindAccessibilityService(res, id); } catch (RemoteException e) { @@ -106,6 +142,15 @@ final class IInputMethodClientInvoker { @AnyThread void onUnbindMethod(int sequence, int unbindReason) { + if (mIsProxy) { + onUnbindMethodInternal(sequence, unbindReason); + } else { + mHandler.post(() -> onUnbindMethodInternal(sequence, unbindReason)); + } + } + + @AnyThread + private void onUnbindMethodInternal(int sequence, int unbindReason) { try { mTarget.onUnbindMethod(sequence, unbindReason); } catch (RemoteException e) { @@ -115,6 +160,15 @@ final class IInputMethodClientInvoker { @AnyThread void onUnbindAccessibilityService(int sequence, int id) { + if (mIsProxy) { + onUnbindAccessibilityServiceInternal(sequence, id); + } else { + mHandler.post(() -> onUnbindAccessibilityServiceInternal(sequence, id)); + } + } + + @AnyThread + private void onUnbindAccessibilityServiceInternal(int sequence, int id) { try { mTarget.onUnbindAccessibilityService(sequence, id); } catch (RemoteException e) { @@ -124,6 +178,16 @@ final class IInputMethodClientInvoker { @AnyThread void setActive(boolean active, boolean fullscreen, boolean reportToImeController) { + if (mIsProxy) { + setActiveInternal(active, fullscreen, reportToImeController); + } else { + mHandler.post(() -> setActiveInternal(active, fullscreen, reportToImeController)); + } + } + + @AnyThread + private void setActiveInternal(boolean active, boolean fullscreen, + boolean reportToImeController) { try { mTarget.setActive(active, fullscreen, reportToImeController); } catch (RemoteException e) { @@ -133,6 +197,15 @@ final class IInputMethodClientInvoker { @AnyThread void scheduleStartInputIfNecessary(boolean fullscreen) { + if (mIsProxy) { + scheduleStartInputIfNecessaryInternal(fullscreen); + } else { + mHandler.post(() -> scheduleStartInputIfNecessaryInternal(fullscreen)); + } + } + + @AnyThread + private void scheduleStartInputIfNecessaryInternal(boolean fullscreen) { try { mTarget.scheduleStartInputIfNecessary(fullscreen); } catch (RemoteException e) { @@ -142,6 +215,15 @@ final class IInputMethodClientInvoker { @AnyThread void reportFullscreenMode(boolean fullscreen) { + if (mIsProxy) { + reportFullscreenModeInternal(fullscreen); + } else { + mHandler.post(() -> reportFullscreenModeInternal(fullscreen)); + } + } + + @AnyThread + private void reportFullscreenModeInternal(boolean fullscreen) { try { mTarget.reportFullscreenMode(fullscreen); } catch (RemoteException e) { @@ -151,6 +233,17 @@ final class IInputMethodClientInvoker { @AnyThread void updateVirtualDisplayToScreenMatrix(int bindSequence, float[] matrixValues) { + if (mIsProxy) { + updateVirtualDisplayToScreenMatrixInternal(bindSequence, matrixValues); + } else { + mHandler.post(() -> + updateVirtualDisplayToScreenMatrixInternal(bindSequence, matrixValues)); + } + } + + @AnyThread + private void updateVirtualDisplayToScreenMatrixInternal(int bindSequence, + float[] matrixValues) { try { mTarget.updateVirtualDisplayToScreenMatrix(bindSequence, matrixValues); } catch (RemoteException e) { @@ -160,6 +253,15 @@ final class IInputMethodClientInvoker { @AnyThread void setImeTraceEnabled(boolean enabled) { + if (mIsProxy) { + setImeTraceEnabledInternal(enabled); + } else { + mHandler.post(() -> setImeTraceEnabledInternal(enabled)); + } + } + + @AnyThread + private void setImeTraceEnabledInternal(boolean enabled) { try { mTarget.setImeTraceEnabled(enabled); } catch (RemoteException e) { @@ -169,6 +271,15 @@ final class IInputMethodClientInvoker { @AnyThread void throwExceptionFromSystem(String message) { + if (mIsProxy) { + throwExceptionFromSystemInternal(message); + } else { + mHandler.post(() -> throwExceptionFromSystemInternal(message)); + } + } + + @AnyThread + private void throwExceptionFromSystemInternal(String message) { try { mTarget.throwExceptionFromSystem(message); } catch (RemoteException e) { diff --git a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java index 9c89563d2c925..3d6b89b729d26 100644 --- a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java +++ b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java @@ -244,13 +244,7 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub private static final int MSG_START_HANDWRITING = 1100; private static final int MSG_FINISH_HANDWRITING = 1110; - private static final int MSG_UNBIND_CLIENT = 3000; - private static final int MSG_UNBIND_ACCESSIBILITY_SERVICE = 3001; - private static final int MSG_BIND_CLIENT = 3010; - private static final int MSG_BIND_ACCESSIBILITY_SERVICE = 3011; - private static final int MSG_SET_ACTIVE = 3020; private static final int MSG_SET_INTERACTIVE = 3030; - private static final int MSG_REPORT_FULLSCREEN_MODE = 3045; private static final int MSG_HARD_KEYBOARD_SWITCH_CHANGED = 4000; @@ -488,10 +482,11 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub + " pid=" + pid + " displayId=" + selfReportedDisplayId + "}"; } - ClientState(IInputMethodClient _client, IRemoteInputConnection _fallbackInputConnection, + ClientState(IInputMethodClientInvoker _client, + IRemoteInputConnection _fallbackInputConnection, int _uid, int _pid, int _selfReportedDisplayId, ClientDeathRecipient _clientDeathRecipient) { - client = IInputMethodClientInvoker.create(_client); + client = _client; fallbackInputConnection = _fallbackInputConnection; uid = _uid; pid = _pid; @@ -2452,8 +2447,10 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub // have the client crash. Thus we do not verify the display ID at all here. Instead we // later check the display ID every time the client needs to interact with the specified // display. - mClients.put(client.asBinder(), new ClientState(client, inputConnection, callerUid, - callerPid, selfReportedDisplayId, deathRecipient)); + final IInputMethodClientInvoker clientInvoker = + IInputMethodClientInvoker.create(client, mHandler); + mClients.put(client.asBinder(), new ClientState(clientInvoker, inputConnection, + callerUid, callerPid, selfReportedDisplayId, deathRecipient)); } } @@ -2497,57 +2494,6 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub } } - @NonNull - private Message obtainMessageOO(int what, Object arg1, Object arg2) { - final SomeArgs args = SomeArgs.obtain(); - args.arg1 = arg1; - args.arg2 = arg2; - return mHandler.obtainMessage(what, 0, 0, args); - } - - @NonNull - private Message obtainMessageOOO(int what, Object arg1, Object arg2, Object arg3) { - SomeArgs args = SomeArgs.obtain(); - args.arg1 = arg1; - args.arg2 = arg2; - args.arg3 = arg3; - return mHandler.obtainMessage(what, 0, 0, args); - } - - @NonNull - private Message obtainMessageIIOO(int what, int arg1, int arg2, - Object arg3, Object arg4) { - SomeArgs args = SomeArgs.obtain(); - args.arg1 = arg3; - args.arg2 = arg4; - return mHandler.obtainMessage(what, arg1, arg2, args); - } - - @NonNull - private Message obtainMessageIIIO(int what, int argi1, int argi2, int argi3, Object arg1) { - final SomeArgs args = SomeArgs.obtain(); - args.arg1 = arg1; - args.argi1 = argi1; - args.argi2 = argi2; - args.argi3 = argi3; - return mHandler.obtainMessage(what, 0, 0, args); - } - - private void executeOrSendMessage(IInputMethodClientInvoker target, Message msg) { - if (target.asBinder() instanceof Binder) { - // This is supposed to be emulating the one-way semantics when the IME client is - // system_server itself, which has not been explicitly prohibited so far while we have - // never ever officially supported such a use case... - // We probably should create a simple wrapper of IInputMethodClient as the first step - // to get rid of executeOrSendMessage() then should prohibit system_server to be the - // IME client for long term. - msg.sendToTarget(); - } else { - handleMessage(msg); - msg.recycle(); - } - } - @GuardedBy("ImfLock.class") void unbindCurrentClientLocked(@UnbindReason int unbindClientReason) { if (mCurClient != null) { @@ -2565,11 +2511,9 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub // Since we set active false to current client and set mCurClient to null, let's unbind // all accessibility too. That means, when input method get disconnected (including // switching ime), we also unbind accessibility - scheduleSetActiveToClient(mCurClient, false /* active */, false /* fullscreen */, + mCurClient.client.setActive(false /* active */, false /* fullscreen */, false /* reportToImeController */); - executeOrSendMessage(mCurClient.client, mHandler.obtainMessage( - MSG_UNBIND_CLIENT, getSequenceNumberLocked(), unbindClientReason, - mCurClient.client)); + mCurClient.client.onUnbindMethod(getSequenceNumberLocked(), unbindClientReason); mCurClient.sessionRequested = false; mCurClient.mSessionRequestedForAccessibility = false; mCurClient = null; @@ -2901,7 +2845,7 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub unbindCurrentClientLocked(UnbindReason.SWITCH_CLIENT); // If the screen is on, inform the new client it is active if (mIsInteractive) { - scheduleSetActiveToClient(cs, true /* active */, false /* fullscreen */, + cs.client.setActive(true /* active */, false /* fullscreen */, false /* reportToImeController */); } } @@ -3020,8 +2964,7 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub attachNewAccessibilityLocked(StartInputReason.SESSION_CREATED_BY_IME, true, -1); if (res.method != null) { - executeOrSendMessage(mCurClient.client, obtainMessageOO( - MSG_BIND_CLIENT, mCurClient.client, res)); + mCurClient.client.onBindMethod(res); } return; } @@ -4529,7 +4472,7 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub public void reportVirtualDisplayGeometryAsync(IInputMethodClient parentClient, int childDisplayId, float[] matrixValues) { final IInputMethodClientInvoker parentClientInvoker = - IInputMethodClientInvoker.create(parentClient); + IInputMethodClientInvoker.create(parentClient, mHandler); try { final DisplayInfo displayInfo = mDisplayManagerInternal.getDisplayInfo(childDisplayId); if (displayInfo == null) { @@ -4997,54 +4940,9 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub // --------------------------------------------------------- - case MSG_UNBIND_CLIENT: { - // This unbinds all accessibility services too. - ((IInputMethodClientInvoker) msg.obj).onUnbindMethod(msg.arg1, msg.arg2); - return true; - } - case MSG_UNBIND_ACCESSIBILITY_SERVICE: { - args = (SomeArgs) msg.obj; - IInputMethodClientInvoker client = (IInputMethodClientInvoker) args.arg1; - int id = (int) args.arg2; - client.onUnbindAccessibilityService(msg.arg1, id); - args.recycle(); - return true; - } - case MSG_BIND_CLIENT: { - args = (SomeArgs)msg.obj; - IInputMethodClientInvoker client = (IInputMethodClientInvoker) args.arg1; - InputBindResult res = (InputBindResult) args.arg2; - client.onBindMethod(res); - args.recycle(); - return true; - } - case MSG_BIND_ACCESSIBILITY_SERVICE: { - args = (SomeArgs) msg.obj; - IInputMethodClientInvoker client = (IInputMethodClientInvoker) args.arg1; - InputBindResult res = (InputBindResult) args.arg2; - int id = (int) args.arg3; - client.onBindAccessibilityService(res, id); - args.recycle(); - return true; - } - case MSG_SET_ACTIVE: { - args = (SomeArgs) msg.obj; - final ClientState clientState = (ClientState) args.arg1; - clientState.client.setActive(args.argi1 != 0 /* active */, - args.argi2 != 0 /* fullScreen */, - args.argi3 != 0 /* reportToImeController */); - args.recycle(); - return true; - } case MSG_SET_INTERACTIVE: handleSetInteractive(msg.arg1 != 0); return true; - case MSG_REPORT_FULLSCREEN_MODE: { - final boolean fullscreen = msg.arg1 != 0; - final ClientState clientState = (ClientState)msg.obj; - clientState.client.reportFullscreenMode(fullscreen); - return true; - } // -------------------------------------------------------------- case MSG_HARD_KEYBOARD_SWITCH_CHANGED: @@ -5138,19 +5036,13 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub // Inform the current client of the change in active status if (mCurClient != null && mCurClient.client != null) { - scheduleSetActiveToClient(mCurClient, mIsInteractive, mInFullscreenMode, + mCurClient.client.setActive(mIsInteractive, mInFullscreenMode, mImePlatformCompatUtils.shouldFinishInputWithReportToIme( getCurMethodUidLocked())); } } } - private void scheduleSetActiveToClient(ClientState state, boolean active, boolean fullscreen, - boolean reportToImeController) { - executeOrSendMessage(state.client, obtainMessageIIIO(MSG_SET_ACTIVE, - active ? 1 : 0, fullscreen ? 1 : 0, reportToImeController ? 1 : 0, state)); - } - @GuardedBy("ImfLock.class") private boolean chooseNewDefaultIMELocked() { final InputMethodInfo imi = InputMethodUtils.getMostApplicableDefaultIME( @@ -5724,9 +5616,7 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub InputBindResult res = attachNewAccessibilityLocked( StartInputReason.SESSION_CREATED_BY_ACCESSIBILITY, true, accessibilityConnectionId); - executeOrSendMessage(mCurClient.client, obtainMessageOOO( - MSG_BIND_ACCESSIBILITY_SERVICE, mCurClient.client, res, - accessibilityConnectionId)); + mCurClient.client.onBindAccessibilityService(res, accessibilityConnectionId); } } } @@ -5741,9 +5631,8 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub } // A11yManagerService unbinds the disabled accessibility service. We don't need // to do it here. - executeOrSendMessage(mCurClient.client, obtainMessageIIOO( - MSG_UNBIND_ACCESSIBILITY_SERVICE, getSequenceNumberLocked(), - 0 /* unused */, mCurClient.client, accessibilityConnectionId)); + mCurClient.client.onUnbindAccessibilityService(getSequenceNumberLocked(), + accessibilityConnectionId); } // We only have sessions when we bound to an input method. Remove this session // from all clients. @@ -5837,9 +5726,7 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub } if (mCurClient != null && mCurClient.client != null) { mInFullscreenMode = fullscreen; - executeOrSendMessage(mCurClient.client, mHandler.obtainMessage( - MSG_REPORT_FULLSCREEN_MODE, fullscreen ? 1 : 0, 0 /* unused */, - mCurClient)); + mCurClient.client.reportFullscreenMode(fullscreen); } } }