From b3c3cae188b7c816b545e8f437f4a96086a70c10 Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Mon, 22 Mar 2021 18:30:08 -0700 Subject: [PATCH] IMMS#dump() should do OOP dumps only for normal prioirty dump This CL reworks our previous CL [1], which entirely moved IMMS dump into critical section in bugreport as a side effect while achieving Bug 167948910. What's unexpected in that CL is that InputMethodManagerService#dump() also tries to extract data from InputMethodService process and IME client process by using Binder IPC. Unfortunately such operations aren't guaranteed to complete within the given timeout for critical dump, which is currently set to 500msec. As a result, we sometimes completely lose the log from those two processes when they take some time to complete. With this CL, InputMethodManagerService#dump() will be registered for both critical dump and normal dump, and those potentially expensive out-of-process dumps will be performed only for normal dump phase like we did before that CL. [1]: Ie87eb8423e2bb70f28c330983d45b95e2e07062d ac24994aaa5bd1f70608cb8c72b93b5ae00d90eb Fix: 177462676 Test: Manually verified that bugreport contains IMMS section of: * Critical text dump without dump from IMS and IME client. * Critical proto dump. * Normal text dump with dump from IMS and IME client. * Normal proto dump. Change-Id: I3828f8eab5739939edabccaa5c76320567f9d7c1 --- .../InputMethodManagerService.java | 76 +++++++++++++++++-- 1 file changed, 68 insertions(+), 8 deletions(-) diff --git a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java index b0b810e5c51d5..10c7a34d8b61b 100644 --- a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java +++ b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java @@ -17,6 +17,7 @@ package com.android.server.inputmethod; import static android.inputmethodservice.InputMethodService.FINISH_INPUT_NO_FALLBACK_CONNECTION; import static android.os.IServiceManager.DUMP_FLAG_PRIORITY_CRITICAL; +import static android.os.IServiceManager.DUMP_FLAG_PRIORITY_NORMAL; import static android.os.IServiceManager.DUMP_FLAG_PROTO; import static android.os.Trace.TRACE_TAG_WINDOW_MANAGER; import static android.server.inputmethod.InputMethodManagerServiceProto.ACCESSIBILITY_REQUESTING_NO_SOFT_KEYBOARD; @@ -178,7 +179,6 @@ import com.android.internal.notification.SystemNotificationChannels; import com.android.internal.os.HandlerCaller; import com.android.internal.os.SomeArgs; import com.android.internal.os.TransferPipe; -import com.android.internal.util.ArrayUtils; import com.android.internal.util.DumpUtils; import com.android.internal.view.IInlineSuggestionsRequestCallback; import com.android.internal.view.IInlineSuggestionsResponseCallback; @@ -198,6 +198,7 @@ import com.android.server.inputmethod.InputMethodSubtypeSwitchingController.ImeS import com.android.server.inputmethod.InputMethodUtils.InputMethodSettings; import com.android.server.pm.UserManagerInternal; import com.android.server.statusbar.StatusBarManagerService; +import com.android.server.utils.PriorityDump; import com.android.server.wm.WindowManagerInternal; import java.io.FileDescriptor; @@ -1565,7 +1566,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub LocalServices.addService(InputMethodManagerInternal.class, new LocalServiceImpl(mService)); publishBinderService(Context.INPUT_METHOD_SERVICE, mService, false /*allowIsolated*/, - DUMP_FLAG_PRIORITY_CRITICAL | DUMP_FLAG_PROTO); + DUMP_FLAG_PRIORITY_CRITICAL | DUMP_FLAG_PRIORITY_NORMAL | DUMP_FLAG_PROTO); } @Override @@ -5224,17 +5225,71 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } } + private final PriorityDump.PriorityDumper mPriorityDumper = new PriorityDump.PriorityDumper() { + /** + * {@inheritDoc} + */ + @BinderThread + @Override + public void dumpCritical(FileDescriptor fd, PrintWriter pw, String[] args, + boolean asProto) { + if (asProto) { + dumpAsProtoNoCheck(fd); + } else { + dumpAsStringNoCheck(fd, pw, args, true /* isCritical */); + } + } + + /** + * {@inheritDoc} + */ + @BinderThread + @Override + public void dumpHigh(FileDescriptor fd, PrintWriter pw, String[] args, boolean asProto) { + dumpNormal(fd, pw, args, asProto); + } + + /** + * {@inheritDoc} + */ + @BinderThread + @Override + public void dumpNormal(FileDescriptor fd, PrintWriter pw, String[] args, boolean asProto) { + if (asProto) { + dumpAsProtoNoCheck(fd); + } else { + dumpAsStringNoCheck(fd, pw, args, false /* isCritical */); + } + } + + /** + * {@inheritDoc} + */ + @BinderThread + @Override + public void dump(FileDescriptor fd, PrintWriter pw, String[] args, boolean asProto) { + dumpNormal(fd, pw, args, asProto); + } + + @BinderThread + private void dumpAsProtoNoCheck(FileDescriptor fd) { + final ProtoOutputStream proto = new ProtoOutputStream(fd); + dumpDebug(proto, InputMethodManagerServiceTraceProto.INPUT_METHOD_MANAGER_SERVICE); + proto.flush(); + } + }; + + @BinderThread @Override protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) { if (!DumpUtils.checkDumpPermission(mContext, TAG, pw)) return; - if (ArrayUtils.contains(args, PROTO_ARG)) { - final ProtoOutputStream proto = new ProtoOutputStream(fd); - dumpDebug(proto, InputMethodManagerServiceTraceProto.INPUT_METHOD_MANAGER_SERVICE); - proto.flush(); - return; - } + PriorityDump.dump(mPriorityDumper, fd, pw, args); + } + @BinderThread + private void dumpAsStringNoCheck(FileDescriptor fd, PrintWriter pw, String[] args, + boolean isCritical) { IInputMethod method; ClientState client; ClientState focusedWindowClient; @@ -5298,6 +5353,11 @@ public class InputMethodManagerService extends IInputMethodManager.Stub mSoftInputShowHideHistory.dump(pw, " "); } + // Exit here for critical dump, as remaining sections require IPCs to other processes. + if (isCritical) { + return; + } + p.println(" "); if (client != null) { pw.flush();