Defer service requests if the app hasn't finished startup

If an app is still in the 'pending finish attach' state, we should
not schedule any service requests to the app because:
1. Pending requests will be dispatched as soon as its startup completes
2. Even if we dispatch a request, it doesn't actually expedite the request
because it will be blocked on the apps main thread which is currently
busy starting up.
3. Lastly and most importantly, this could lead to multiple onCreate requests
to the same service when the app completes startup if there were multiple bind
requests prior to its startup.

This is easy to repro with a test app that sleeps in the Application#onCreate
while a bindService is sent multiple times during the sleep.

Test: Manual (CTS pending)
Bug: 289688173
Change-Id: Iedaa62354418428a98b612067dd6439daf9dd530
This commit is contained in:
Zim
2023-08-01 19:13:43 +01:00
parent 77bfa87487
commit beef3189ea
2 changed files with 12 additions and 7 deletions

View File

@@ -5067,7 +5067,7 @@ public final class ActiveServices {
boolean whileRestarting, boolean permissionsReviewRequired, boolean packageFrozen,
boolean enqueueOomAdj)
throws TransactionTooLargeException {
if (r.app != null && r.app.getThread() != null) {
if (r.app != null && r.app.isThreadReady()) {
sendServiceArgsLocked(r, execInFg, false);
return null;
}
@@ -5139,7 +5139,7 @@ public final class ActiveServices {
final IApplicationThread thread = app.getThread();
final int pid = app.getPid();
final UidRecord uidRecord = app.getUidRecord();
if (thread != null) {
if (app.isThreadReady()) {
try {
if (Trace.isTagEnabled(Trace.TRACE_TAG_ACTIVITY_MANAGER)) {
Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER,
@@ -5171,7 +5171,7 @@ public final class ActiveServices {
final int pid = app.getPid();
final UidRecord uidRecord = app.getUidRecord();
r.isolationHostProc = app;
if (thread != null) {
if (app.isThreadReady()) {
try {
if (Trace.isTagEnabled(Trace.TRACE_TAG_ACTIVITY_MANAGER)) {
Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER,
@@ -5571,7 +5571,7 @@ public final class ActiveServices {
boolean oomAdjusted = false;
// Tell the service that it has been unbound.
if (r.app != null && r.app.getThread() != null) {
if (r.app != null && r.app.isThreadReady()) {
for (int i = r.bindings.size() - 1; i >= 0; i--) {
IntentBindRecord ibr = r.bindings.valueAt(i);
if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Bringing down binding " + ibr
@@ -5713,7 +5713,7 @@ public final class ActiveServices {
mAm.mBatteryStatsService.noteServiceStopLaunch(r.appInfo.uid, r.name.getPackageName(),
r.name.getClassName());
stopServiceAndUpdateAllowlistManagerLocked(r);
if (r.app.getThread() != null) {
if (r.app.isThreadReady()) {
// Bump the process to the top of LRU list
mAm.updateLruProcessLocked(r.app, false, null);
updateServiceForegroundLocked(r.app.mServices, false);
@@ -5877,7 +5877,7 @@ public final class ActiveServices {
if (!c.serviceDead) {
if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Disconnecting binding " + b.intent
+ ": shouldUnbind=" + b.intent.hasBound);
if (s.app != null && s.app.getThread() != null && b.intent.apps.size() == 0
if (s.app != null && s.app.isThreadReady() && b.intent.apps.size() == 0
&& b.intent.hasBound) {
try {
bumpServiceExecutingLocked(s, false, "unbind", OOM_ADJ_REASON_UNBIND_SERVICE);
@@ -6379,7 +6379,7 @@ public final class ActiveServices {
sr.pendingStarts.add(new ServiceRecord.StartItem(sr, true,
sr.getLastStartId(), baseIntent, null, 0, null, null,
ActivityManager.PROCESS_STATE_UNKNOWN));
if (sr.app != null && sr.app.getThread() != null) {
if (sr.app != null && sr.app.isThreadReady()) {
// We always run in the foreground, since this is called as
// part of the "remove task" UI operation.
try {

View File

@@ -769,6 +769,11 @@ class ProcessRecord implements WindowProcessListener {
return mPendingFinishAttach;
}
@GuardedBy("mService")
boolean isThreadReady() {
return mThread != null && !mPendingFinishAttach;
}
@GuardedBy("mService")
long getStartSeq() {
return mStartSeq;