From 484d03d55c8cbd2079cd5d7fe797d389847ccafc Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Mon, 7 Nov 2022 10:57:22 -0700 Subject: [PATCH] BroadcastQueue: blocked registered receivers. In most situations, it's reasonable to shortcut and "assumeDelivered" for registered receivers as a fire-and-forget operation, since if that app is wedged it'll only impact delivery of future broadcasts to that app. However, for ordered and resultTo broadcasts, there are often other apps that need to be "blocked" until the receiver actually finishes. If we "assumeDelivered" in those cases, we'd be subjecting those other apps to race conditions. To resolve this, we tighten "assumeDelivered" for these situations, which also ensure we detect ANRs that would block delivery for other apps, and add tests to verify. Bug: 257972988 Test: atest FrameworksMockingServicesTests:BroadcastRecordTest Test: atest FrameworksMockingServicesTests:BroadcastQueueTest Test: atest FrameworksMockingServicesTests:BroadcastQueueModernImplTest Change-Id: I492aa9ff6f7797e64245c2073db8e2ecddfd8544 --- core/java/android/app/ActivityThread.java | 34 ++-- core/java/android/app/IApplicationThread.aidl | 6 +- core/java/android/app/LoadedApk.java | 55 +++---- core/java/android/app/ReceiverInfo.aidl | 1 + .../android/content/BroadcastReceiver.java | 41 ++++- .../android/server/am/BroadcastQueueImpl.java | 8 +- .../server/am/BroadcastQueueModernImpl.java | 19 ++- .../server/am/BroadcastReceiverBatch.java | 19 ++- .../am/SameProcessApplicationThread.java | 18 +-- .../android/server/am/BroadcastQueueTest.java | 145 ++++++++++++++---- 10 files changed, 241 insertions(+), 105 deletions(-) diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index 86482c013a141..89789f4b2045e 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -784,9 +784,10 @@ public final class ActivityThread extends ClientTransactionHandler static final class ReceiverData extends BroadcastReceiver.PendingResult { public ReceiverData(Intent intent, int resultCode, String resultData, Bundle resultExtras, - boolean ordered, boolean sticky, IBinder token, int sendingUser) { + boolean ordered, boolean sticky, boolean assumeDelivered, IBinder token, + int sendingUser) { super(resultCode, resultData, resultExtras, TYPE_COMPONENT, ordered, sticky, - token, sendingUser, intent.getFlags()); + assumeDelivered, token, sendingUser, intent.getFlags()); this.intent = intent; } @@ -1040,10 +1041,10 @@ public final class ActivityThread extends ClientTransactionHandler public final void scheduleReceiver(Intent intent, ActivityInfo info, CompatibilityInfo compatInfo, int resultCode, String data, Bundle extras, - boolean sync, int sendingUser, int processState) { + boolean ordered, boolean assumeDelivered, int sendingUser, int processState) { updateProcessState(processState, false); ReceiverData r = new ReceiverData(intent, resultCode, data, extras, - sync, false, mAppThread.asBinder(), sendingUser); + ordered, false, assumeDelivered, mAppThread.asBinder(), sendingUser); r.info = info; sendMessage(H.RECEIVER, r); } @@ -1054,11 +1055,11 @@ public final class ActivityThread extends ClientTransactionHandler if (r.registered) { scheduleRegisteredReceiver(r.receiver, r.intent, r.resultCode, r.data, r.extras, r.ordered, r.sticky, - r.sendingUser, r.processState); + r.assumeDelivered, r.sendingUser, r.processState); } else { scheduleReceiver(r.intent, r.activityInfo, r.compatInfo, r.resultCode, r.data, r.extras, r.sync, - r.sendingUser, r.processState); + r.assumeDelivered, r.sendingUser, r.processState); } } } @@ -1288,10 +1289,25 @@ public final class ActivityThread extends ClientTransactionHandler // applies transaction ordering per object for such calls. public void scheduleRegisteredReceiver(IIntentReceiver receiver, Intent intent, int resultCode, String dataStr, Bundle extras, boolean ordered, - boolean sticky, int sendingUser, int processState) throws RemoteException { + boolean sticky, boolean assumeDelivered, int sendingUser, int processState) + throws RemoteException { updateProcessState(processState, false); - receiver.performReceive(intent, resultCode, dataStr, extras, ordered, - sticky, sendingUser); + + // We can't modify IIntentReceiver due to UnsupportedAppUsage, so + // try our best to shortcut to known subclasses, and alert if + // registered using a custom IIntentReceiver that isn't able to + // report an expected delivery event + if (receiver instanceof LoadedApk.ReceiverDispatcher.InnerReceiver) { + ((LoadedApk.ReceiverDispatcher.InnerReceiver) receiver).performReceive(intent, + resultCode, dataStr, extras, ordered, sticky, assumeDelivered, sendingUser); + } else { + if (!assumeDelivered) { + Log.wtf(TAG, "scheduleRegisteredReceiver() called for " + receiver + + " and " + intent + " without mechanism to finish delivery"); + } + receiver.performReceive(intent, resultCode, dataStr, extras, ordered, sticky, + sendingUser); + } } @Override diff --git a/core/java/android/app/IApplicationThread.aidl b/core/java/android/app/IApplicationThread.aidl index 3984fee192037..4f69d85fdbdb0 100644 --- a/core/java/android/app/IApplicationThread.aidl +++ b/core/java/android/app/IApplicationThread.aidl @@ -65,8 +65,8 @@ import java.util.Map; oneway interface IApplicationThread { void scheduleReceiver(in Intent intent, in ActivityInfo info, in CompatibilityInfo compatInfo, - int resultCode, in String data, in Bundle extras, boolean sync, - int sendingUser, int processState); + int resultCode, in String data, in Bundle extras, boolean ordered, + boolean assumeDelivered, int sendingUser, int processState); void scheduleReceiverList(in List info); @@ -102,7 +102,7 @@ oneway interface IApplicationThread { in String[] args); void scheduleRegisteredReceiver(IIntentReceiver receiver, in Intent intent, int resultCode, in String data, in Bundle extras, boolean ordered, - boolean sticky, int sendingUser, int processState); + boolean sticky, boolean assumeDelivered, int sendingUser, int processState); void scheduleLowMemory(); void profilerControl(boolean start, in ProfilerInfo profilerInfo, int profileType); void setSchedulingGroup(int group); diff --git a/core/java/android/app/LoadedApk.java b/core/java/android/app/LoadedApk.java index 3620a601d355f..7c22902a25a6d 100644 --- a/core/java/android/app/LoadedApk.java +++ b/core/java/android/app/LoadedApk.java @@ -1679,6 +1679,16 @@ public final class LoadedApk { @Override public void performReceive(Intent intent, int resultCode, String data, Bundle extras, boolean ordered, boolean sticky, int sendingUser) { + Log.wtf(TAG, "performReceive() called targeting raw IIntentReceiver for " + intent); + performReceive(intent, resultCode, data, extras, ordered, sticky, + BroadcastReceiver.PendingResult.guessAssumeDelivered( + BroadcastReceiver.PendingResult.TYPE_REGISTERED, ordered), + sendingUser); + } + + public void performReceive(Intent intent, int resultCode, String data, + Bundle extras, boolean ordered, boolean sticky, boolean assumeDelivered, + int sendingUser) { final LoadedApk.ReceiverDispatcher rd; if (intent == null) { Log.wtf(TAG, "Null intent received"); @@ -1693,8 +1703,8 @@ public final class LoadedApk { } if (rd != null) { rd.performReceive(intent, resultCode, data, extras, - ordered, sticky, sendingUser); - } else { + ordered, sticky, assumeDelivered, sendingUser); + } else if (!assumeDelivered) { // The activity manager dispatched a broadcast to a registered // receiver in this process, but before it could be delivered the // receiver was unregistered. Acknowledge the broadcast on its @@ -1729,30 +1739,26 @@ public final class LoadedApk { final class Args extends BroadcastReceiver.PendingResult { private Intent mCurIntent; - private final boolean mOrdered; private boolean mDispatched; private boolean mRunCalled; public Args(Intent intent, int resultCode, String resultData, Bundle resultExtras, - boolean ordered, boolean sticky, int sendingUser) { + boolean ordered, boolean sticky, boolean assumeDelivered, int sendingUser) { super(resultCode, resultData, resultExtras, mRegistered ? TYPE_REGISTERED : TYPE_UNREGISTERED, ordered, - sticky, mAppThread.asBinder(), sendingUser, intent.getFlags()); + sticky, assumeDelivered, mAppThread.asBinder(), sendingUser, + intent.getFlags()); mCurIntent = intent; - mOrdered = ordered; } public final Runnable getRunnable() { return () -> { final BroadcastReceiver receiver = mReceiver; - final boolean ordered = mOrdered; if (ActivityThread.DEBUG_BROADCAST) { int seq = mCurIntent.getIntExtra("seq", -1); Slog.i(ActivityThread.TAG, "Dispatching broadcast " + mCurIntent.getAction() + " seq=" + seq + " to " + mReceiver); - Slog.i(ActivityThread.TAG, " mRegistered=" + mRegistered - + " mOrderedHint=" + ordered); } final IActivityManager mgr = ActivityManager.getService(); @@ -1766,11 +1772,9 @@ public final class LoadedApk { mDispatched = true; mRunCalled = true; if (receiver == null || intent == null || mForgotten) { - if (mRegistered && ordered) { - if (ActivityThread.DEBUG_BROADCAST) Slog.i(ActivityThread.TAG, - "Finishing null broadcast to " + mReceiver); - sendFinished(mgr); - } + if (ActivityThread.DEBUG_BROADCAST) Slog.i(ActivityThread.TAG, + "Finishing null broadcast to " + mReceiver); + sendFinished(mgr); return; } @@ -1790,11 +1794,9 @@ public final class LoadedApk { receiver.setPendingResult(this); receiver.onReceive(mContext, intent); } catch (Exception e) { - if (mRegistered && ordered) { - if (ActivityThread.DEBUG_BROADCAST) Slog.i(ActivityThread.TAG, - "Finishing failed broadcast to " + mReceiver); - sendFinished(mgr); - } + if (ActivityThread.DEBUG_BROADCAST) Slog.i(ActivityThread.TAG, + "Finishing failed broadcast to " + mReceiver); + sendFinished(mgr); if (mInstrumentation == null || !mInstrumentation.onException(mReceiver, e)) { Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); @@ -1868,9 +1870,10 @@ public final class LoadedApk { } public void performReceive(Intent intent, int resultCode, String data, - Bundle extras, boolean ordered, boolean sticky, int sendingUser) { + Bundle extras, boolean ordered, boolean sticky, boolean assumeDelivered, + int sendingUser) { final Args args = new Args(intent, resultCode, data, extras, ordered, - sticky, sendingUser); + sticky, assumeDelivered, sendingUser); if (intent == null) { Log.wtf(TAG, "Null intent received"); } else { @@ -1881,12 +1884,10 @@ public final class LoadedApk { } } if (intent == null || !mActivityThread.post(args.getRunnable())) { - if (mRegistered && ordered) { - IActivityManager mgr = ActivityManager.getService(); - if (ActivityThread.DEBUG_BROADCAST) Slog.i(ActivityThread.TAG, - "Finishing sync broadcast to " + mReceiver); - args.sendFinished(mgr); - } + IActivityManager mgr = ActivityManager.getService(); + if (ActivityThread.DEBUG_BROADCAST) Slog.i(ActivityThread.TAG, + "Finishing sync broadcast to " + mReceiver); + args.sendFinished(mgr); } } diff --git a/core/java/android/app/ReceiverInfo.aidl b/core/java/android/app/ReceiverInfo.aidl index d90eee704a7e5..8d7e3c4bcaae1 100644 --- a/core/java/android/app/ReceiverInfo.aidl +++ b/core/java/android/app/ReceiverInfo.aidl @@ -34,6 +34,7 @@ parcelable ReceiverInfo { Intent intent; String data; Bundle extras; + boolean assumeDelivered; int sendingUser; int processState; int resultCode; diff --git a/core/java/android/content/BroadcastReceiver.java b/core/java/android/content/BroadcastReceiver.java index c7a3b52097af2..64dcc4d1687d3 100644 --- a/core/java/android/content/BroadcastReceiver.java +++ b/core/java/android/content/BroadcastReceiver.java @@ -82,6 +82,7 @@ public abstract class BroadcastReceiver { final boolean mOrderedHint; @UnsupportedAppUsage final boolean mInitialStickyHint; + final boolean mAssumeDeliveredHint; @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023) final IBinder mToken; @UnsupportedAppUsage @@ -105,17 +106,38 @@ public abstract class BroadcastReceiver { @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023) public PendingResult(int resultCode, String resultData, Bundle resultExtras, int type, boolean ordered, boolean sticky, IBinder token, int userId, int flags) { + this(resultCode, resultData, resultExtras, type, ordered, sticky, + guessAssumeDelivered(type, ordered), token, userId, flags); + } + + /** @hide */ + public PendingResult(int resultCode, String resultData, Bundle resultExtras, int type, + boolean ordered, boolean sticky, boolean assumeDelivered, IBinder token, + int userId, int flags) { mResultCode = resultCode; mResultData = resultData; mResultExtras = resultExtras; mType = type; mOrderedHint = ordered; mInitialStickyHint = sticky; + mAssumeDeliveredHint = assumeDelivered; mToken = token; mSendingUser = userId; mFlags = flags; } + /** @hide */ + public static boolean guessAssumeDelivered(int type, boolean ordered) { + // When a caller didn't provide a concrete way of knowing if we need + // to report delivery, make a best-effort guess + if (type == TYPE_COMPONENT) { + return false; + } else if (ordered && type != TYPE_UNREGISTERED) { + return false; + } + return true; + } + /** * Version of {@link BroadcastReceiver#setResultCode(int) * BroadcastReceiver.setResultCode(int)} for @@ -252,7 +274,7 @@ public abstract class BroadcastReceiver { "Finishing broadcast to component " + mToken); sendFinished(mgr); } - } else if (mOrderedHint && mType != TYPE_UNREGISTERED) { + } else { if (ActivityThread.DEBUG_BROADCAST) Slog.i(ActivityThread.TAG, "Finishing broadcast to " + mToken); final IActivityManager mgr = ActivityManager.getService(); @@ -279,13 +301,16 @@ public abstract class BroadcastReceiver { if (mResultExtras != null) { mResultExtras.setAllowFds(false); } - if (mOrderedHint) { - am.finishReceiver(mToken, mResultCode, mResultData, mResultExtras, - mAbortBroadcast, mFlags); - } else { - // This broadcast was sent to a component; it is not ordered, - // but we still need to tell the activity manager we are done. - am.finishReceiver(mToken, 0, null, null, false, mFlags); + + // When the OS didn't assume delivery, we need to inform + // it that we've actually finished the delivery + if (!mAssumeDeliveredHint) { + if (mOrderedHint) { + am.finishReceiver(mToken, mResultCode, mResultData, mResultExtras, + mAbortBroadcast, mFlags); + } else { + am.finishReceiver(mToken, 0, null, null, false, mFlags); + } } } catch (RemoteException ex) { } diff --git a/services/core/java/com/android/server/am/BroadcastQueueImpl.java b/services/core/java/com/android/server/am/BroadcastQueueImpl.java index 8946ada1daf97..a86efa3877974 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueImpl.java @@ -397,11 +397,12 @@ public class BroadcastQueueImpl extends BroadcastQueue { + ": " + r); mService.notifyPackageUse(r.intent.getComponent().getPackageName(), PackageManager.NOTIFY_PACKAGE_USE_BROADCAST_RECEIVER); + final boolean assumeDelivered = false; thread.scheduleReceiverList(mReceiverBatch.manifestReceiver( prepareReceiverIntent(r.intent, r.curFilteredExtras), r.curReceiver, null /* compatInfo (unused but need to keep method signature) */, - r.resultCode, r.resultData, r.resultExtras, r.ordered, r.userId, - app.mState.getReportedProcState())); + r.resultCode, r.resultData, r.resultExtras, r.ordered, assumeDelivered, + r.userId, app.mState.getReportedProcState())); if (DEBUG_BROADCAST) Slog.v(TAG_BROADCAST, "Process cur broadcast " + r + " DELIVERED for app " + app); started = true; @@ -735,9 +736,10 @@ public class BroadcastQueueImpl extends BroadcastQueue { // If we have an app thread, do the call through that so it is // correctly ordered with other one-way calls. try { + final boolean assumeDelivered = !ordered; thread.scheduleReceiverList(mReceiverBatch.registeredReceiver( receiver, intent, resultCode, - data, extras, ordered, sticky, sendingUser, + data, extras, ordered, sticky, assumeDelivered, sendingUser, app.mState.getReportedProcState())); } catch (RemoteException ex) { // Failed to call into the process. It's either dying or wedged. Kill it gently. diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index a994b1db7ca33..7e413fba7bab1 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -872,21 +872,26 @@ class BroadcastQueueModernImpl extends BroadcastQueue { return true; } + final boolean assumeDelivered = isAssumedDelivered(r, index); if (receiver instanceof BroadcastFilter) { batch.schedule(((BroadcastFilter) receiver).receiverList.receiver, receiverIntent, r.resultCode, r.resultData, r.resultExtras, - r.ordered, r.initialSticky, r.userId, + r.ordered, r.initialSticky, assumeDelivered, r.userId, app.mState.getReportedProcState(), r, index); // TODO: consider making registered receivers of unordered // broadcasts report results to detect ANRs - if (!r.ordered) { + if (assumeDelivered) { batch.success(r, index, BroadcastRecord.DELIVERY_DELIVERED, "assuming delivered"); return true; } } else { batch.schedule(receiverIntent, ((ResolveInfo) receiver).activityInfo, - null, r.resultCode, r.resultData, r.resultExtras, r.ordered, r.userId, - app.mState.getReportedProcState(), r, index); + null, r.resultCode, r.resultData, r.resultExtras, r.ordered, assumeDelivered, + r.userId, app.mState.getReportedProcState(), r, index); + if (assumeDelivered) { + batch.success(r, index, BroadcastRecord.DELIVERY_DELIVERED, "assuming delivered"); + return true; + } } return false; @@ -975,7 +980,8 @@ class BroadcastQueueModernImpl extends BroadcastQueue { * Return true if this receiver should be assumed to have been delivered. */ private boolean isAssumedDelivered(BroadcastRecord r, int index) { - return (r.receivers.get(index) instanceof BroadcastFilter) && !r.ordered; + return (r.receivers.get(index) instanceof BroadcastFilter) && !r.ordered + && (r.resultTo == null); } /** @@ -1034,10 +1040,11 @@ class BroadcastQueueModernImpl extends BroadcastQueue { mService.mOomAdjuster.mCachedAppOptimizer.unfreezeTemporarily( app, OOM_ADJ_REASON_FINISH_RECEIVER); try { + final boolean assumeDelivered = true; thread.scheduleReceiverList(mReceiverBatch.registeredReceiver( r.resultTo, r.intent, r.resultCode, r.resultData, r.resultExtras, false, r.initialSticky, - r.userId, app.mState.getReportedProcState())); + assumeDelivered, r.userId, app.mState.getReportedProcState())); } catch (RemoteException e) { final String msg = "Failed to schedule result of " + r + " via " + app + ": " + e; logw(msg); diff --git a/services/core/java/com/android/server/am/BroadcastReceiverBatch.java b/services/core/java/com/android/server/am/BroadcastReceiverBatch.java index a8264582b8b36..226647c77ca3e 100644 --- a/services/core/java/com/android/server/am/BroadcastReceiverBatch.java +++ b/services/core/java/com/android/server/am/BroadcastReceiverBatch.java @@ -19,8 +19,8 @@ package com.android.server.am; import android.annotation.NonNull; import android.annotation.Nullable; import android.app.ReceiverInfo; -import android.content.Intent; import android.content.IIntentReceiver; +import android.content.Intent; import android.content.pm.ActivityInfo; import android.content.res.CompatibilityInfo; import android.os.Bundle; @@ -171,12 +171,13 @@ final class BroadcastReceiverBatch { // Add a ReceiverInfo for a registered receiver. void schedule(@Nullable IIntentReceiver receiver, Intent intent, int resultCode, @Nullable String data, @Nullable Bundle extras, boolean ordered, - boolean sticky, int sendingUser, int processState, + boolean sticky, boolean assumeDelivered, int sendingUser, int processState, @Nullable BroadcastRecord r, int index) { ReceiverInfo ri = new ReceiverInfo(); ri.intent = intent; ri.data = data; ri.extras = extras; + ri.assumeDelivered = assumeDelivered; ri.sendingUser = sendingUser; ri.processState = processState; ri.resultCode = resultCode; @@ -190,12 +191,13 @@ final class BroadcastReceiverBatch { // Add a ReceiverInfo for a manifest receiver. void schedule(@Nullable Intent intent, @Nullable ActivityInfo activityInfo, @Nullable CompatibilityInfo compatInfo, int resultCode, @Nullable String data, - @Nullable Bundle extras, boolean sync, int sendingUser, int processState, - @Nullable BroadcastRecord r, int index) { + @Nullable Bundle extras, boolean sync, boolean assumeDelivered, int sendingUser, + int processState, @Nullable BroadcastRecord r, int index) { ReceiverInfo ri = new ReceiverInfo(); ri.intent = intent; ri.data = data; ri.extras = extras; + ri.assumeDelivered = assumeDelivered; ri.sendingUser = sendingUser; ri.processState = processState; ri.resultCode = resultCode; @@ -214,10 +216,10 @@ final class BroadcastReceiverBatch { */ ArrayList registeredReceiver(@Nullable IIntentReceiver receiver, @Nullable Intent intent, int resultCode, @Nullable String data, - @Nullable Bundle extras, boolean ordered, boolean sticky, + @Nullable Bundle extras, boolean ordered, boolean sticky, boolean assumeDelivered, int sendingUser, int processState) { reset(); - schedule(receiver, intent, resultCode, data, extras, ordered, sticky, + schedule(receiver, intent, resultCode, data, extras, ordered, sticky, assumeDelivered, sendingUser, processState, null, 0); return receivers(); } @@ -225,9 +227,9 @@ final class BroadcastReceiverBatch { ArrayList manifestReceiver(@Nullable Intent intent, @Nullable ActivityInfo activityInfo, @Nullable CompatibilityInfo compatInfo, int resultCode, @Nullable String data, @Nullable Bundle extras, boolean sync, - int sendingUser, int processState) { + boolean assumeDelivered, int sendingUser, int processState) { reset(); - schedule(intent, activityInfo, compatInfo, resultCode, data, extras, sync, + schedule(intent, activityInfo, compatInfo, resultCode, data, extras, sync, assumeDelivered, sendingUser, processState, null, 0); return receivers(); } @@ -255,6 +257,7 @@ final class BroadcastReceiverBatch { n.intent = r.intent; n.data = r.data; n.extras = r.extras; + n.assumeDelivered = r.assumeDelivered; n.sendingUser = r.sendingUser; n.processState = r.processState; n.resultCode = r.resultCode; diff --git a/services/core/java/com/android/server/am/SameProcessApplicationThread.java b/services/core/java/com/android/server/am/SameProcessApplicationThread.java index 62fd6e9d551b8..082e8e04479bb 100644 --- a/services/core/java/com/android/server/am/SameProcessApplicationThread.java +++ b/services/core/java/com/android/server/am/SameProcessApplicationThread.java @@ -47,12 +47,12 @@ public class SameProcessApplicationThread extends IApplicationThread.Default { @Override public void scheduleReceiver(Intent intent, ActivityInfo info, CompatibilityInfo compatInfo, - int resultCode, String data, Bundle extras, boolean sync, int sendingUser, - int processState) { + int resultCode, String data, Bundle extras, boolean ordered, boolean assumeDelivered, + int sendingUser, int processState) { mHandler.post(() -> { try { - mWrapped.scheduleReceiver(intent, info, compatInfo, resultCode, data, extras, sync, - sendingUser, processState); + mWrapped.scheduleReceiver(intent, info, compatInfo, resultCode, data, extras, + ordered, assumeDelivered, sendingUser, processState); } catch (RemoteException e) { throw new RuntimeException(e); } @@ -61,12 +61,12 @@ public class SameProcessApplicationThread extends IApplicationThread.Default { @Override public void scheduleRegisteredReceiver(IIntentReceiver receiver, Intent intent, int resultCode, - String data, Bundle extras, boolean ordered, boolean sticky, int sendingUser, - int processState) { + String data, Bundle extras, boolean ordered, boolean sticky, boolean assumeDelivered, + int sendingUser, int processState) { mHandler.post(() -> { try { mWrapped.scheduleRegisteredReceiver(receiver, intent, resultCode, data, extras, - ordered, sticky, sendingUser, processState); + ordered, sticky, assumeDelivered, sendingUser, processState); } catch (RemoteException e) { throw new RuntimeException(e); } @@ -79,11 +79,11 @@ public class SameProcessApplicationThread extends IApplicationThread.Default { ReceiverInfo r = info.get(i); if (r.registered) { scheduleRegisteredReceiver(r.receiver, r.intent, - r.resultCode, r.data, r.extras, r.ordered, r.sticky, + r.resultCode, r.data, r.extras, r.ordered, r.sticky, r.assumeDelivered, r.sendingUser, r.processState); } else { scheduleReceiver(r.intent, r.activityInfo, r.compatInfo, - r.resultCode, r.data, r.extras, r.sync, + r.resultCode, r.data, r.extras, r.sync, r.assumeDelivered, r.sendingUser, r.processState); } } diff --git a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java index 8d9dda0863e92..6d525d18283db 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java @@ -92,6 +92,7 @@ import com.android.server.appop.AppOpsService; import com.android.server.wm.ActivityTaskManagerService; import org.junit.After; +import org.junit.Assume; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -442,9 +443,9 @@ public class BroadcastQueueTest { UnaryOperator extrasOperator, ReceiverInfo info) { final Intent intent = info.intent; final Bundle extras = info.extras; - final boolean ordered = info.ordered; + final boolean assumeDelivered = info.assumeDelivered; mScheduledBroadcasts.add(makeScheduledBroadcast(r, intent)); - if (!wedge && ordered) { + if (!wedge && !assumeDelivered) { assertTrue(r.mReceivers.numberOfCurReceivers() > 0); assertNotEquals(ProcessList.SCHED_GROUP_UNDEFINED, mQueue.getPreferredSchedulingGroupLocked(r)); @@ -695,6 +696,7 @@ public class BroadcastQueueTest { ArgumentMatcher data, ArgumentMatcher extras, Boolean sync, + Boolean assumeDelivered, Integer sendingUser, Integer processState) { return (test) -> { @@ -706,6 +708,7 @@ public class BroadcastQueueTest { && matchObject(data, test.data) && matchObject(extras, test.extras) && matchElement(sync, test.sync) + && matchElement(assumeDelivered, test.assumeDelivered) && matchElement(sendingUser, test.sendingUser) && matchElement(processState, test.processState); }; @@ -724,10 +727,12 @@ public class BroadcastQueueTest { ArgumentMatcher data, ArgumentMatcher extras, Boolean sync, + Boolean assumeDelivered, Integer sendingUser, Integer processState) { return argThat(receiverList(manifestReceiverMatcher(intent, activityInfo, compatInfo, - resultCode, data, extras, sync, sendingUser, processState))); + resultCode, data, extras, sync, assumeDelivered, + sendingUser, processState))); } /** @@ -743,6 +748,7 @@ public class BroadcastQueueTest { ArgumentMatcher extras, Boolean ordered, Boolean sticky, + Boolean assumeDelivered, Integer sendingUser, Integer processState) { return (test) -> { @@ -754,6 +760,7 @@ public class BroadcastQueueTest { && matchObject(extras, test.extras) && matchElement(ordered, test.ordered) && matchElement(sticky, test.sticky) + && matchElement(assumeDelivered, test.assumeDelivered) && matchElement(sendingUser, test.sendingUser) && matchElement(processState, test.processState); }; @@ -771,10 +778,12 @@ public class BroadcastQueueTest { ArgumentMatcher extras, Boolean ordered, Boolean sticky, + Boolean assumeDelivered, Integer sendingUser, Integer processState) { return argThat(receiverList(registeredReceiverMatcher(receiver, intent, resultCode, - data, extras, ordered, sticky, sendingUser, processState))); + data, extras, ordered, sticky, assumeDelivered, + sendingUser, processState))); } /** @@ -827,36 +836,36 @@ public class BroadcastQueueTest { final Intent targetedIntent = new Intent(intent); targetedIntent.setComponent(component); verify(app.getThread(), mode).scheduleReceiverList( - manifestReceiver(filterEquals(targetedIntent), - null, null, null, null, null, null, UserHandle.USER_SYSTEM, null)); + manifestReceiver(filterEquals(targetedIntent), + null, null, null, null, null, null, null, UserHandle.USER_SYSTEM, null)); } private void verifyScheduleReceiver(VerificationMode mode, ProcessRecord app, Intent intent, int userId) throws Exception { verify(app.getThread(), mode).scheduleReceiverList( - manifestReceiver(filterEqualsIgnoringComponent(intent), - null, null, null, null, null, null, userId, null)); + manifestReceiver(filterEqualsIgnoringComponent(intent), + null, null, null, null, null, null, null, userId, null)); } private void verifyScheduleReceiver(VerificationMode mode, ProcessRecord app, int userId) throws Exception { verify(app.getThread(), mode).scheduleReceiverList( manifestReceiver(null, - null, null, null, null, null, null, userId, null)); + null, null, null, null, null, null, null, userId, null)); } private void verifyScheduleRegisteredReceiver(ProcessRecord app, Intent intent) throws Exception { verify(app.getThread()).scheduleReceiverList( - registeredReceiver(null, filterEqualsIgnoringComponent(intent), - null, null, null, null, null, UserHandle.USER_SYSTEM, null)); + registeredReceiver(null, filterEqualsIgnoringComponent(intent), + null, null, null, null, null, null, UserHandle.USER_SYSTEM, null)); } private void verifyScheduleRegisteredReceiver(VerificationMode mode, ProcessRecord app, int userId) throws Exception { verify(app.getThread(), mode).scheduleReceiverList( registeredReceiver(null, null, - null, null, null, null, null, userId, null)); + null, null, null, null, null, null, userId, null)); } static final int USER_GUEST = 11; @@ -1112,10 +1121,11 @@ public class BroadcastQueueTest { } /** - * Verify that we detect and ANR a wedged process. + * Verify that we detect and ANR a wedged process when delivering to a + * manifest receiver. */ @Test - public void testWedged() throws Exception { + public void testWedged_Manifest() throws Exception { final ProcessRecord callerApp = makeActiveProcessRecord(PACKAGE_RED); final ProcessRecord receiverApp = makeActiveProcessRecord(PACKAGE_GREEN, ProcessBehavior.WEDGE); @@ -1128,6 +1138,77 @@ public class BroadcastQueueTest { verify(mAms).appNotResponding(eq(receiverApp), any()); } + /** + * Verify that we detect and ANR a wedged process when delivering an ordered + * broadcast, and that we deliver final result. + */ + @Test + public void testWedged_Registered_Ordered() throws Exception { + // Legacy stack doesn't detect these ANRs; likely an oversight + Assume.assumeTrue(mImpl == Impl.MODERN); + + final ProcessRecord callerApp = makeActiveProcessRecord(PACKAGE_RED); + final ProcessRecord receiverApp = makeActiveProcessRecord(PACKAGE_GREEN, + ProcessBehavior.WEDGE); + + final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED); + final IIntentReceiver resultTo = mock(IIntentReceiver.class); + enqueueBroadcast(makeOrderedBroadcastRecord(airplane, callerApp, + List.of(makeRegisteredReceiver(receiverApp)), resultTo, null)); + + waitForIdle(); + verify(mAms).appNotResponding(eq(receiverApp), any()); + verifyScheduleRegisteredReceiver(callerApp, airplane); + } + + /** + * Verify that we detect and ANR a wedged process when delivering an + * unordered broadcast with a {@code resultTo}. + */ + @Test + public void testWedged_Registered_ResultTo() throws Exception { + // Legacy stack doesn't detect these ANRs; likely an oversight + Assume.assumeTrue(mImpl == Impl.MODERN); + + final ProcessRecord callerApp = makeActiveProcessRecord(PACKAGE_RED); + final ProcessRecord receiverApp = makeActiveProcessRecord(PACKAGE_GREEN, + ProcessBehavior.WEDGE); + + final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED); + final IIntentReceiver resultTo = mock(IIntentReceiver.class); + enqueueBroadcast(makeBroadcastRecord(airplane, callerApp, + List.of(makeRegisteredReceiver(receiverApp)), resultTo)); + + waitForIdle(); + verify(mAms).appNotResponding(eq(receiverApp), any()); + verifyScheduleRegisteredReceiver(callerApp, airplane); + } + + /** + * Verify that we detect and ANR a wedged process when delivering a + * broadcast with more than one priority tranche. + */ + @Test + public void testWedged_Registered_Prioritized() throws Exception { + // Legacy stack doesn't detect these ANRs; likely an oversight + Assume.assumeTrue(mImpl == Impl.MODERN); + + final ProcessRecord callerApp = makeActiveProcessRecord(PACKAGE_RED); + final ProcessRecord receiverGreenApp = makeActiveProcessRecord(PACKAGE_GREEN, + ProcessBehavior.WEDGE); + final ProcessRecord receiverBlueApp = makeActiveProcessRecord(PACKAGE_BLUE, + ProcessBehavior.NORMAL); + + final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED); + enqueueBroadcast(makeBroadcastRecord(airplane, callerApp, + List.of(makeRegisteredReceiver(receiverGreenApp, 10), + makeRegisteredReceiver(receiverBlueApp, 5)))); + + waitForIdle(); + verify(mAms).appNotResponding(eq(receiverGreenApp), any()); + verifyScheduleRegisteredReceiver(receiverBlueApp, airplane); + } + /** * Verify that we handle registered receivers in a process that always * responds with {@link DeadObjectException}, recovering to restart the @@ -1338,11 +1419,11 @@ public class BroadcastQueueTest { // Confirm that we saw no registered receiver traffic final IApplicationThread oldThread = oldApp.getThread(); - verify(oldThread, never()).scheduleRegisteredReceiver(any(), - any(), anyInt(), any(), any(), anyBoolean(), anyBoolean(), anyInt(), anyInt()); + verify(oldThread, never()).scheduleRegisteredReceiver(any(), any(), anyInt(), any(), any(), + anyBoolean(), anyBoolean(), anyBoolean(), anyInt(), anyInt()); final IApplicationThread newThread = newApp.getThread(); - verify(newThread, never()).scheduleRegisteredReceiver(any(), - any(), anyInt(), any(), any(), anyBoolean(), anyBoolean(), anyInt(), anyInt()); + verify(newThread, never()).scheduleRegisteredReceiver(any(), any(), anyInt(), any(), any(), + anyBoolean(), anyBoolean(), anyBoolean(), anyInt(), anyInt()); // Confirm that we saw final manifest broadcast verifyScheduleReceiver(times(1), newApp, airplane, @@ -1464,22 +1545,22 @@ public class BroadcastQueueTest { expectedExtras.putBoolean(PACKAGE_RED, true); inOrder.verify(greenThread).scheduleReceiverList(manifestReceiver( filterEqualsIgnoringComponent(airplane), null, null, - Activity.RESULT_OK, null, bundleEquals(expectedExtras), true, + Activity.RESULT_OK, null, bundleEquals(expectedExtras), true, false, UserHandle.USER_SYSTEM, null)); inOrder.verify(blueThread).scheduleReceiverList(manifestReceiver( filterEqualsIgnoringComponent(airplane), null, null, - Activity.RESULT_OK, null, bundleEquals(expectedExtras), true, + Activity.RESULT_OK, null, bundleEquals(expectedExtras), true, false, UserHandle.USER_SYSTEM, null)); expectedExtras.putBoolean(PACKAGE_BLUE, true); inOrder.verify(yellowThread).scheduleReceiverList(manifestReceiver( filterEqualsIgnoringComponent(airplane), null, null, - Activity.RESULT_OK, null, bundleEquals(expectedExtras), true, + Activity.RESULT_OK, null, bundleEquals(expectedExtras), true, false, UserHandle.USER_SYSTEM, null)); expectedExtras.putBoolean(PACKAGE_YELLOW, true); inOrder.verify(redThread).scheduleReceiverList(registeredReceiver( null, filterEquals(airplane), Activity.RESULT_OK, null, bundleEquals(expectedExtras), false, - null, UserHandle.USER_SYSTEM, null)); + null, true, UserHandle.USER_SYSTEM, null)); // Finally, verify that we thawed the final receiver verify(mAms.mOomAdjuster.mCachedAppOptimizer).unfreezeTemporarily(eq(callerApp), @@ -1544,22 +1625,22 @@ public class BroadcastQueueTest { final InOrder inOrder = inOrder(greenThread, blueThread, redThread); inOrder.verify(greenThread).scheduleReceiverList(manifestReceiver( filterEqualsIgnoringComponent(intent), null, null, - Activity.RESULT_OK, null, null, true, UserHandle.USER_SYSTEM, + Activity.RESULT_OK, null, null, true, false, UserHandle.USER_SYSTEM, null)); if ((intent.getFlags() & Intent.FLAG_RECEIVER_NO_ABORT) != 0) { inOrder.verify(blueThread).scheduleReceiverList(manifestReceiver( filterEqualsIgnoringComponent(intent), null, null, - Activity.RESULT_OK, null, null, true, UserHandle.USER_SYSTEM, + Activity.RESULT_OK, null, null, true, false, UserHandle.USER_SYSTEM, null)); } else { inOrder.verify(blueThread, never()).scheduleReceiverList(manifestReceiver( - null, null, null, null, + null, null, null, null, null, null, null, null, null, null)); } inOrder.verify(redThread).scheduleReceiverList(registeredReceiver( null, filterEquals(intent), Activity.RESULT_OK, null, bundleEquals(expectedExtras), - false, null, UserHandle.USER_SYSTEM, null)); + false, null, true, UserHandle.USER_SYSTEM, null)); } /** @@ -1582,7 +1663,7 @@ public class BroadcastQueueTest { verify(callerThread).scheduleReceiverList(registeredReceiver( null, filterEquals(airplane), Activity.RESULT_OK, null, bundleEquals(orderedExtras), false, - null, UserHandle.USER_SYSTEM, null)); + null, true, UserHandle.USER_SYSTEM, null)); } /** @@ -1603,7 +1684,7 @@ public class BroadcastQueueTest { verify(callerThread).scheduleReceiverList(registeredReceiver( null, filterEquals(airplane), Activity.RESULT_OK, null, null, false, - null, UserHandle.USER_SYSTEM, null)); + null, true, UserHandle.USER_SYSTEM, null)); } /** @@ -1768,26 +1849,26 @@ public class BroadcastQueueTest { // First broadcast is canceled inOrder.verify(callerThread).scheduleReceiverList(registeredReceiver(null, filterAndExtrasEquals(timezoneFirst), Activity.RESULT_CANCELED, null, - null, false, null, UserHandle.USER_SYSTEM, null)); + null, false, null, true, UserHandle.USER_SYSTEM, null)); // We deliver second broadcast to app timezoneSecond.setClassName(PACKAGE_BLUE, CLASS_GREEN); inOrder.verify(blueThread).scheduleReceiverList(manifestReceiver( filterAndExtrasEquals(timezoneSecond), - null, null, null, null, null, true, null, null)); + null, null, null, null, null, true, false, null, null)); // Second broadcast is finished timezoneSecond.setComponent(null); inOrder.verify(callerThread).scheduleReceiverList(registeredReceiver(null, filterAndExtrasEquals(timezoneSecond), Activity.RESULT_OK, null, - null, false, null, UserHandle.USER_SYSTEM, null)); + null, false, null, true, UserHandle.USER_SYSTEM, null)); // Since we "replaced" the first broadcast in its original position, // only now do we see the airplane broadcast airplane.setClassName(PACKAGE_BLUE, CLASS_RED); inOrder.verify(blueThread).scheduleReceiverList(manifestReceiver( filterEquals(airplane), - null, null, null, null, null, false, null, null)); + null, null, null, null, null, false, false, null, null)); } @Test