From 3f7c9f2d0164f2b5826c194e9309791637f35c2c Mon Sep 17 00:00:00 2001 From: Dianne Hackborn Date: Tue, 4 Apr 2017 15:36:33 -0700 Subject: [PATCH] Deliver start service args with ParcelledListSlice. We have seen issues where we fail restarting a process because there are so many services with so many pending start arguments that we hit IPC limits. So instead of doing an IPC for each service start, collect them together in a list that is sent once, and send it inside of a ParcelledListSlice to control how much data is written inline in the IPC. Test: boot and ran Change-Id: Ifed26ccdf535871e577fc02c7ef1d09060ab06ca --- core/java/android/app/ActivityThread.java | 23 +-- core/java/android/app/IApplicationThread.aidl | 4 +- core/java/android/app/ServiceStartArgs.aidl | 20 +++ core/java/android/app/ServiceStartArgs.java | 82 ++++++++++ .../content/pm/BaseParceledListSlice.java | 12 +- .../com/android/server/am/ActiveServices.java | 150 ++++++++++-------- 6 files changed, 209 insertions(+), 82 deletions(-) create mode 100644 core/java/android/app/ServiceStartArgs.aidl create mode 100644 core/java/android/app/ServiceStartArgs.java diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index e49aad2cd607c..d8e574274bea6 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -38,6 +38,7 @@ import android.content.pm.InstrumentationInfo; import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.content.pm.PackageManager.NameNotFoundException; +import android.content.pm.ParceledListSlice; import android.content.pm.ProviderInfo; import android.content.pm.ServiceInfo; import android.content.res.AssetManager; @@ -868,16 +869,20 @@ public final class ActivityThread { sendMessage(H.UNBIND_SERVICE, s); } - public final void scheduleServiceArgs(IBinder token, boolean taskRemoved, int startId, - int flags ,Intent args) { - ServiceArgsData s = new ServiceArgsData(); - s.token = token; - s.taskRemoved = taskRemoved; - s.startId = startId; - s.flags = flags; - s.args = args; + public final void scheduleServiceArgs(IBinder token, ParceledListSlice args) { + List list = args.getList(); - sendMessage(H.SERVICE_ARGS, s); + for (int i = 0; i < list.size(); i++) { + ServiceStartArgs ssa = list.get(i); + ServiceArgsData s = new ServiceArgsData(); + s.token = token; + s.taskRemoved = ssa.taskRemoved; + s.startId = ssa.startId; + s.flags = ssa.flags; + s.args = ssa.args; + + sendMessage(H.SERVICE_ARGS, s); + } } public final void scheduleStopService(IBinder token) { diff --git a/core/java/android/app/IApplicationThread.aidl b/core/java/android/app/IApplicationThread.aidl index 6c43fe3beca17..1b3c00b615397 100644 --- a/core/java/android/app/IApplicationThread.aidl +++ b/core/java/android/app/IApplicationThread.aidl @@ -25,6 +25,7 @@ import android.content.IIntentReceiver; import android.content.Intent; import android.content.pm.ActivityInfo; import android.content.pm.ApplicationInfo; +import android.content.pm.ParceledListSlice; import android.content.pm.ProviderInfo; import android.content.pm.ServiceInfo; import android.content.res.CompatibilityInfo; @@ -86,8 +87,7 @@ oneway interface IApplicationThread { in Bundle coreSettings, in String buildSerial); void scheduleExit(); void scheduleConfigurationChanged(in Configuration config); - void scheduleServiceArgs(IBinder token, boolean taskRemoved, int startId, - int flags, in Intent args); + void scheduleServiceArgs(IBinder token, in ParceledListSlice args); void updateTimeZone(); void processInBackground(); void scheduleBindService(IBinder token, diff --git a/core/java/android/app/ServiceStartArgs.aidl b/core/java/android/app/ServiceStartArgs.aidl new file mode 100644 index 0000000000000..fd2cf0f93dc63 --- /dev/null +++ b/core/java/android/app/ServiceStartArgs.aidl @@ -0,0 +1,20 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.app; + +/** @hide */ +parcelable ServiceStartArgs; diff --git a/core/java/android/app/ServiceStartArgs.java b/core/java/android/app/ServiceStartArgs.java new file mode 100644 index 0000000000000..f030cbaaecafd --- /dev/null +++ b/core/java/android/app/ServiceStartArgs.java @@ -0,0 +1,82 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.app; + +import android.content.Intent; +import android.os.Parcel; +import android.os.Parcelable; + +/** + * Describes a Service.onStartCommand() request from the system. + * @hide + */ +public class ServiceStartArgs implements Parcelable { + final public boolean taskRemoved; + final public int startId; + final public int flags; + final public Intent args; + + public ServiceStartArgs(boolean _taskRemoved, int _startId, int _flags, Intent _args) { + taskRemoved = _taskRemoved; + startId = _startId; + flags = _flags; + args = _args; + } + + public String toString() { + return "ServiceStartArgs{taskRemoved=" + taskRemoved + ", startId=" + startId + + ", flags=0x" + Integer.toHexString(flags) + ", args=" + args + "}"; + } + + public int describeContents() { + return 0; + } + + public void writeToParcel(Parcel out, int flags) { + out.writeInt(taskRemoved ? 1 : 0); + out.writeInt(startId); + out.writeInt(flags); + if (args != null) { + out.writeInt(1); + args.writeToParcel(out, 0); + } else { + out.writeInt(0); + } + } + + public static final Parcelable.Creator CREATOR + = new Parcelable.Creator() { + public ServiceStartArgs createFromParcel(Parcel in) { + return new ServiceStartArgs(in); + } + + public ServiceStartArgs[] newArray(int size) { + return new ServiceStartArgs[size]; + } + }; + + public ServiceStartArgs(Parcel in) { + taskRemoved = in.readInt() != 0; + startId = in.readInt(); + flags = in.readInt(); + if (in.readInt() != 0) { + args = Intent.CREATOR.createFromParcel(in); + } else { + args = null; + } + } +} diff --git a/core/java/android/content/pm/BaseParceledListSlice.java b/core/java/android/content/pm/BaseParceledListSlice.java index c4e4e06be7497..aaa5f19c3fca3 100644 --- a/core/java/android/content/pm/BaseParceledListSlice.java +++ b/core/java/android/content/pm/BaseParceledListSlice.java @@ -50,6 +50,8 @@ abstract class BaseParceledListSlice implements Parcelable { private final List mList; + private int mInlineCountLimit = Integer.MAX_VALUE; + public BaseParceledListSlice(List list) { mList = list; } @@ -134,6 +136,14 @@ abstract class BaseParceledListSlice implements Parcelable { return mList; } + /** + * Set a limit on the maximum number of entries in the array that will be included + * inline in the initial parcelling of this object. + */ + public void setInlineCountLimit(int maxCount) { + mInlineCountLimit = maxCount; + } + /** * Write this to another Parcel. Note that this discards the internal Parcel * and should not be used anymore. This is so we can pass this to a Binder @@ -149,7 +159,7 @@ abstract class BaseParceledListSlice implements Parcelable { final Class listElementClass = mList.get(0).getClass(); writeParcelableCreator(mList.get(0), dest); int i = 0; - while (i < N && dest.dataSize() < MAX_IPC_SIZE) { + while (i < N && i < mInlineCountLimit && dest.dataSize() < MAX_IPC_SIZE) { dest.writeInt(1); final T parcelable = mList.get(i); diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java index 4cbfb275fd32d..bebcd4a120796 100644 --- a/services/core/java/com/android/server/am/ActiveServices.java +++ b/services/core/java/com/android/server/am/ActiveServices.java @@ -31,8 +31,10 @@ import java.util.Set; import android.app.ActivityThread; import android.app.AppOpsManager; +import android.app.ServiceStartArgs; import android.content.IIntentSender; import android.content.IntentSender; +import android.content.pm.ParceledListSlice; import android.os.Build; import android.os.Bundle; import android.os.DeadObjectException; @@ -1956,78 +1958,86 @@ public final class ActiveServices { return; } - while (r.pendingStarts.size() > 0) { - Exception caughtException = null; - ServiceRecord.StartItem si = null; - try { - si = r.pendingStarts.remove(0); - if (DEBUG_SERVICE) { - Slog.v(TAG_SERVICE, "Sending arguments to: " - + r + " " + r.intent + " args=" + si.intent); - } - if (si.intent == null && N > 1) { - // If somehow we got a dummy null intent in the middle, - // then skip it. DO NOT skip a null intent when it is - // the only one in the list -- this is to support the - // onStartCommand(null) case. - continue; - } - si.deliveredTime = SystemClock.uptimeMillis(); - r.deliveredStarts.add(si); - si.deliveryCount++; - if (si.neededGrants != null) { - mAm.grantUriPermissionUncheckedFromIntentLocked(si.neededGrants, - si.getUriPermissionsLocked()); - } - mAm.grantEphemeralAccessLocked(r.userId, si.intent, - r.appInfo.uid, UserHandle.getAppId(si.callingId)); - bumpServiceExecutingLocked(r, execInFg, "start"); - if (!oomAdjusted) { - oomAdjusted = true; - mAm.updateOomAdjLocked(r.app); - } - if (r.fgRequired && !r.fgWaiting) { - if (!r.isForeground) { - if (DEBUG_BACKGROUND_CHECK) { - Slog.i(TAG, "Launched service must call startForeground() within timeout: " + r); - } - scheduleServiceForegroundTransitionTimeoutLocked(r); - } else { - if (DEBUG_BACKGROUND_CHECK) { - Slog.i(TAG, "Service already foreground; no new timeout: " + r); - } - r.fgRequired = false; - } - } - int flags = 0; - if (si.deliveryCount > 1) { - flags |= Service.START_FLAG_RETRY; - } - if (si.doneExecutingCount > 0) { - flags |= Service.START_FLAG_REDELIVERY; - } - r.app.thread.scheduleServiceArgs(r, si.taskRemoved, si.id, flags, si.intent); - } catch (TransactionTooLargeException e) { - if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Transaction too large: intent=" - + si.intent); - caughtException = e; - } catch (RemoteException e) { - // Remote process gone... we'll let the normal cleanup take care of this. - if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Crashed while sending args: " + r); - caughtException = e; - } catch (Exception e) { - Slog.w(TAG, "Unexpected exception", e); - caughtException = e; - } + ArrayList args = new ArrayList<>(); - if (caughtException != null) { - // Keep nesting count correct - final boolean inDestroying = mDestroyingServices.contains(r); - serviceDoneExecutingLocked(r, inDestroying, inDestroying); - if (caughtException instanceof TransactionTooLargeException) { - throw (TransactionTooLargeException)caughtException; + while (r.pendingStarts.size() > 0) { + ServiceRecord.StartItem si = r.pendingStarts.remove(0); + if (DEBUG_SERVICE) { + Slog.v(TAG_SERVICE, "Sending arguments to: " + + r + " " + r.intent + " args=" + si.intent); + } + if (si.intent == null && N > 1) { + // If somehow we got a dummy null intent in the middle, + // then skip it. DO NOT skip a null intent when it is + // the only one in the list -- this is to support the + // onStartCommand(null) case. + continue; + } + si.deliveredTime = SystemClock.uptimeMillis(); + r.deliveredStarts.add(si); + si.deliveryCount++; + if (si.neededGrants != null) { + mAm.grantUriPermissionUncheckedFromIntentLocked(si.neededGrants, + si.getUriPermissionsLocked()); + } + mAm.grantEphemeralAccessLocked(r.userId, si.intent, + r.appInfo.uid, UserHandle.getAppId(si.callingId)); + bumpServiceExecutingLocked(r, execInFg, "start"); + if (!oomAdjusted) { + oomAdjusted = true; + mAm.updateOomAdjLocked(r.app); + } + if (r.fgRequired && !r.fgWaiting) { + if (!r.isForeground) { + if (DEBUG_BACKGROUND_CHECK) { + Slog.i(TAG, "Launched service must call startForeground() within timeout: " + r); + } + scheduleServiceForegroundTransitionTimeoutLocked(r); + } else { + if (DEBUG_BACKGROUND_CHECK) { + Slog.i(TAG, "Service already foreground; no new timeout: " + r); + } + r.fgRequired = false; } - break; + } + int flags = 0; + if (si.deliveryCount > 1) { + flags |= Service.START_FLAG_RETRY; + } + if (si.doneExecutingCount > 0) { + flags |= Service.START_FLAG_REDELIVERY; + } + args.add(new ServiceStartArgs(si.taskRemoved, si.id, flags, si.intent)); + } + + ParceledListSlice slice = new ParceledListSlice<>(args); + slice.setInlineCountLimit(4); + Exception caughtException = null; + try { + r.app.thread.scheduleServiceArgs(r, slice); + } catch (TransactionTooLargeException e) { + if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Transaction too large for " + args.size() + + " args, first: " + args.get(0).args); + Slog.w(TAG, "Failed delivering service starts", e); + caughtException = e; + } catch (RemoteException e) { + // Remote process gone... we'll let the normal cleanup take care of this. + if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Crashed while sending args: " + r); + Slog.w(TAG, "Failed delivering service starts", e); + caughtException = e; + } catch (Exception e) { + Slog.w(TAG, "Unexpected exception", e); + caughtException = e; + } + + if (caughtException != null) { + // Keep nesting count correct + final boolean inDestroying = mDestroyingServices.contains(r); + for (int i = 0; i < args.size(); i++) { + serviceDoneExecutingLocked(r, inDestroying, inDestroying); + } + if (caughtException instanceof TransactionTooLargeException) { + throw (TransactionTooLargeException)caughtException; } } }