From 5719028766f51512beffa623db7ee682851570a0 Mon Sep 17 00:00:00 2001 From: Joe Onorato Date: Wed, 20 Apr 2016 15:37:49 -0700 Subject: [PATCH 1/2] If a crash dialog can't be shown, just kill the process. scheduleCrash() calls into the app process to ask it to nicely show the crash dialog. If that call fails, rather than just silently stopping, kill the process right away. It might be out of control. This also adds an additional check for killedByAm to the flow there so if the activity manager already has killed it, the dialog won't be shown. Bug: 28196243 Change-Id: I979f01074e5890640e7b06e29eeb0d6c38803569 --- .../java/com/android/server/am/AppErrors.java | 14 +---------- .../com/android/server/am/ProcessRecord.java | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/services/core/java/com/android/server/am/AppErrors.java b/services/core/java/com/android/server/am/AppErrors.java index 68bd2fd3ce3e8..e9ed34b18baa4 100644 --- a/services/core/java/com/android/server/am/AppErrors.java +++ b/services/core/java/com/android/server/am/AppErrors.java @@ -293,19 +293,7 @@ class AppErrors { return; } - if (proc.thread != null) { - if (proc.pid == Process.myPid()) { - Log.w(TAG, "crashApplication: trying to crash self!"); - return; - } - long ident = Binder.clearCallingIdentity(); - try { - proc.thread.scheduleCrash(message); - } catch (RemoteException e) { - } finally { - Binder.restoreCallingIdentity(ident); - } - } + proc.scheduleCrash(message); } /** diff --git a/services/core/java/com/android/server/am/ProcessRecord.java b/services/core/java/com/android/server/am/ProcessRecord.java index 93d40604dbda4..059acbd4190f0 100644 --- a/services/core/java/com/android/server/am/ProcessRecord.java +++ b/services/core/java/com/android/server/am/ProcessRecord.java @@ -37,9 +37,11 @@ import android.content.ComponentName; import android.content.Context; import android.content.pm.ApplicationInfo; import android.content.res.CompatibilityInfo; +import android.os.Binder; import android.os.Bundle; import android.os.IBinder; import android.os.Process; +import android.os.RemoteException; import android.os.SystemClock; import android.os.Trace; import android.os.UserHandle; @@ -552,6 +554,29 @@ final class ProcessRecord { return adj; } + void scheduleCrash(String message) { + // Checking killedbyAm should keep it from showing the crash dialog if the process + // was already dead for a good / normal reason. + if (!killedByAm) { + if (thread != null) { + if (pid == Process.myPid()) { + Slog.w(TAG, "scheduleCrash: trying to crash system process!"); + return; + } + long ident = Binder.clearCallingIdentity(); + try { + thread.scheduleCrash(message); + } catch (RemoteException e) { + // If it's already dead our work is done. If it's wedged just kill it. + // We won't get the crash dialog or the error reporting. + kill("scheduleCrash for '" + message + "' failed", true); + } finally { + Binder.restoreCallingIdentity(ident); + } + } + } + } + void kill(String reason, boolean noisy) { if (!killedByAm) { Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "kill"); From 5869d1c5b67b8e8ee22696318ffb31316dcbb089 Mon Sep 17 00:00:00 2001 From: Joe Onorato Date: Wed, 20 Apr 2016 15:38:07 -0700 Subject: [PATCH 2/2] If we can't call into an app process to deliver an intent, crash the process. Rather than hoping the process will crash for some other reason, it is better to just get rid of it right away. The Intent was not delivered, so there are no guarantees that the app will continue to function correctly. Bug: 28196243 Change-Id: I036fbc5ffd42dc7259437cc5a733976478a2dd3a --- .../com/android/server/am/BroadcastQueue.java | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/am/BroadcastQueue.java b/services/core/java/com/android/server/am/BroadcastQueue.java index 37c7765f26e38..bbb8bdf8185e6 100644 --- a/services/core/java/com/android/server/am/BroadcastQueue.java +++ b/services/core/java/com/android/server/am/BroadcastQueue.java @@ -38,6 +38,7 @@ import android.content.pm.PackageManager; import android.content.pm.ResolveInfo; import android.os.Build; import android.os.Bundle; +import android.os.DeadObjectException; import android.os.Handler; import android.os.IBinder; import android.os.Looper; @@ -446,7 +447,7 @@ public final class BroadcastQueue { } } - private static void performReceiveLocked(ProcessRecord app, IIntentReceiver receiver, + private void performReceiveLocked(ProcessRecord app, IIntentReceiver receiver, Intent intent, int resultCode, String data, Bundle extras, boolean ordered, boolean sticky, int sendingUser) throws RemoteException { // Send the intent to the receiver asynchronously using one-way binder calls. @@ -454,8 +455,23 @@ public final class BroadcastQueue { if (app.thread != null) { // If we have an app thread, do the call through that so it is // correctly ordered with other one-way calls. - app.thread.scheduleRegisteredReceiver(receiver, intent, resultCode, - data, extras, ordered, sticky, sendingUser, app.repProcState); + try { + app.thread.scheduleRegisteredReceiver(receiver, intent, resultCode, + data, extras, ordered, sticky, sendingUser, app.repProcState); + // TODO: Uncomment this when (b/28322359) is fixed and we aren't getting + // DeadObjectException when the process isn't actually dead. + //} catch (DeadObjectException ex) { + // Failed to call into the process. It's dying so just let it die and move on. + // throw ex; + } catch (RemoteException ex) { + // Failed to call into the process. It's either dying or wedged. Kill it gently. + synchronized (mService) { + Slog.w(TAG, "Can't deliver broadcast to " + app.processName + + " (pid " + app.pid + "). Crashing it."); + app.scheduleCrash("can't deliver broadcast"); + } + throw ex; + } } else { // Application has died. Receiver doesn't exist. throw new RemoteException("app.thread must not be null"); @@ -853,6 +869,7 @@ public final class BroadcastQueue { Slog.w(TAG, "Failure [" + mQueueName + "] sending broadcast result of " + r.intent, e); + } }