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

2 files changed, 12 insertions, 7 deletions

diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java
index d959de33d3e9..26f2373c44aa 100644
--- a/services/core/java/com/android/server/am/ActiveServices.java
+++ b/services/core/java/com/android/server/am/ActiveServices.java
@@ -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 {
diff --git a/services/core/java/com/android/server/am/ProcessRecord.java b/services/core/java/com/android/server/am/ProcessRecord.java
index f532122c10d9..f6acc41e4df3 100644
--- a/services/core/java/com/android/server/am/ProcessRecord.java
+++ b/services/core/java/com/android/server/am/ProcessRecord.java
@@ -770,6 +770,11 @@ class ProcessRecord implements WindowProcessListener {
     }
 
     @GuardedBy("mService")
+    boolean isThreadReady() {
+        return mThread != null && !mPendingFinishAttach;
+    }
+
+    @GuardedBy("mService")
     long getStartSeq() {
         return mStartSeq;
     }