Merge "Apply proper locks onto the service/association state tracking" into sc-dev am: 365cc28b50

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/15475947

Change-Id: Iec6a0045666cf3bf4545a807a6810247434d6429

services/core/java/com/android/server/am/ActiveServices.java

@@ -807,11 +807,13 @@ public final class ActiveServices {
 
         if (fgRequired) {
             // We are now effectively running a foreground service.
-            ServiceState stracker = r.getTracker();
+            synchronized (mAm.mProcessStats.mLock) {
+                final ServiceState stracker = r.getTracker();
                 if (stracker != null) {
                     stracker.setForeground(true, mAm.mProcessStats.getMemFactorLocked(),
                             r.lastActivity);
                 }
+            }
             mAm.mAppOpsService.startOperation(AppOpsManager.getToken(mAm.mAppOpsService),
                     AppOpsManager.OP_START_FOREGROUND, r.appInfo.uid, r.packageName, null,
                     true, false, null, false, AppOpsManager.ATTRIBUTION_FLAGS_NONE,
@@ -1082,10 +1084,12 @@ public final class ActiveServices {
 
     ComponentName startServiceInnerLocked(ServiceMap smap, Intent service, ServiceRecord r,
             boolean callerFg, boolean addToStarting) throws TransactionTooLargeException {
-        ServiceState stracker = r.getTracker();
+        synchronized (mAm.mProcessStats.mLock) {
+            final ServiceState stracker = r.getTracker();
             if (stracker != null) {
                 stracker.setStarted(true, mAm.mProcessStats.getMemFactorLocked(), r.lastActivity);
             }
+        }
         r.callStart = false;
 
         final int uid = r.appInfo.uid;
@@ -1144,9 +1148,11 @@ public final class ActiveServices {
         mAm.mBatteryStatsService.noteServiceStopRunning(uid, packageName, serviceName);
         service.startRequested = false;
         if (service.tracker != null) {
+            synchronized (mAm.mProcessStats.mLock) {
                 service.tracker.setStarted(false, mAm.mProcessStats.getMemFactorLocked(),
                         SystemClock.uptimeMillis());
             }
+        }
         service.callStart = false;
 
         bringDownServiceIfNeededLocked(service, false, false, enqueueOomAdj);
@@ -1320,9 +1326,11 @@ public final class ActiveServices {
             mAm.mBatteryStatsService.noteServiceStopRunning(uid, packageName, serviceName);
             r.startRequested = false;
             if (r.tracker != null) {
+                synchronized (mAm.mProcessStats.mLock) {
                     r.tracker.setStarted(false, mAm.mProcessStats.getMemFactorLocked(),
                             SystemClock.uptimeMillis());
                 }
+            }
             r.callStart = false;
             final long origId = Binder.clearCallingIdentity();
             bringDownServiceIfNeededLocked(r, false, false, false);
@@ -1877,11 +1885,13 @@ public final class ActiveServices {
                         r.mStartForegroundCount++;
                         r.mFgsEnterTime = SystemClock.uptimeMillis();
                         if (!stopProcStatsOp) {
-                            ServiceState stracker = r.getTracker();
+                            synchronized (mAm.mProcessStats.mLock) {
+                                final ServiceState stracker = r.getTracker();
                                 if (stracker != null) {
                                     stracker.setForeground(true,
                                             mAm.mProcessStats.getMemFactorLocked(), r.lastActivity);
                                 }
+                            }
                         } else {
                             stopProcStatsOp = false;
                         }
@@ -1915,12 +1925,14 @@ public final class ActiveServices {
                 if (stopProcStatsOp) {
                     // We got through to this point with it actively being started foreground,
                     // and never decided we wanted to keep it like that, so drop it.
-                    ServiceState stracker = r.getTracker();
+                    synchronized (mAm.mProcessStats.mLock) {
+                        final ServiceState stracker = r.getTracker();
                         if (stracker != null) {
                             stracker.setForeground(false, mAm.mProcessStats.getMemFactorLocked(),
                                     SystemClock.uptimeMillis());
                         }
                     }
+                }
                 if (alreadyStartedOp) {
                     // If we had previously done a start op for direct foreground start,
                     // we have cleared the flag so can now drop it.
@@ -1960,11 +1972,13 @@ public final class ActiveServices {
 
                 r.isForeground = false;
                 r.mFgsExitTime = SystemClock.uptimeMillis();
-                ServiceState stracker = r.getTracker();
+                synchronized (mAm.mProcessStats.mLock) {
+                    final ServiceState stracker = r.getTracker();
                     if (stracker != null) {
                         stracker.setForeground(false, mAm.mProcessStats.getMemFactorLocked(),
                                 SystemClock.uptimeMillis());
                     }
+                }
                 mAm.mAppOpsService.finishOperation(
                         AppOpsManager.getToken(mAm.mAppOpsService),
                         AppOpsManager.OP_START_FOREGROUND, r.appInfo.uid, r.packageName, null);
@@ -2738,13 +2752,15 @@ public final class ActiveServices {
                 s.lastActivity = SystemClock.uptimeMillis();
                 if (!s.hasAutoCreateConnections()) {
                     // This is the first binding, let the tracker know.
-                    ServiceState stracker = s.getTracker();
+                    synchronized (mAm.mProcessStats.mLock) {
+                        final ServiceState stracker = s.getTracker();
                         if (stracker != null) {
                             stracker.setBound(true, mAm.mProcessStats.getMemFactorLocked(),
                                     s.lastActivity);
                         }
                     }
                 }
+            }
 
             if ((flags & Context.BIND_RESTRICT_ASSOCIATIONS) != 0) {
                 mAm.requireAllowedAssociationsLocked(s.appInfo.packageName);
@@ -3378,10 +3394,12 @@ public final class ActiveServices {
         ProcessServiceRecord psr;
         if (r.executeNesting == 0) {
             r.executeFg = fg;
-            ServiceState stracker = r.getTracker();
+            synchronized (mAm.mProcessStats.mLock) {
+                final ServiceState stracker = r.getTracker();
                 if (stracker != null) {
                     stracker.setExecuting(true, mAm.mProcessStats.getMemFactorLocked(), now);
                 }
+            }
             if (r.app != null) {
                 psr = r.app.mServices;
                 psr.startExecutingService(r);
@@ -3575,8 +3593,10 @@ public final class ActiveServices {
         if (!mRestartingServices.contains(r)) {
             r.createdFromFg = false;
             mRestartingServices.add(r);
+            synchronized (mAm.mProcessStats.mLock) {
                 r.makeRestarting(mAm.mProcessStats.getMemFactorLocked(), now);
             }
+        }
 
         cancelForegroundNotificationLocked(r);
 
@@ -3653,8 +3673,10 @@ public final class ActiveServices {
                 }
             }
             if (!stillTracking) {
+                synchronized (mAm.mProcessStats.mLock) {
                     r.restartTracker.setRestarting(false, mAm.mProcessStats.getMemFactorLocked(),
                             SystemClock.uptimeMillis());
+                }
                 r.restartTracker = null;
             }
         }
@@ -4174,10 +4196,12 @@ public final class ActiveServices {
                     + r);
             r.fgRequired = false;
             r.fgWaiting = false;
+            synchronized (mAm.mProcessStats.mLock) {
                 ServiceState stracker = r.getTracker();
                 if (stracker != null) {
                     stracker.setForeground(false, mAm.mProcessStats.getMemFactorLocked(), now);
                 }
+            }
             mAm.mAppOpsService.finishOperation(AppOpsManager.getToken(mAm.mAppOpsService),
                     AppOpsManager.OP_START_FOREGROUND, r.appInfo.uid, r.packageName, null);
             mAm.mHandler.removeMessages(
@@ -4233,10 +4257,12 @@ public final class ActiveServices {
         cancelForegroundNotificationLocked(r);
         if (r.isForeground) {
             decActiveForegroundAppLocked(smap, r);
+            synchronized (mAm.mProcessStats.mLock) {
                 ServiceState stracker = r.getTracker();
                 if (stracker != null) {
                     stracker.setForeground(false, mAm.mProcessStats.getMemFactorLocked(), now);
                 }
+            }
             mAm.mAppOpsService.finishOperation(
                     AppOpsManager.getToken(mAm.mAppOpsService),
                     AppOpsManager.OP_START_FOREGROUND, r.appInfo.uid, r.packageName, null);
@@ -4303,7 +4329,8 @@ public final class ActiveServices {
            ((ServiceRestarter)r.restarter).setService(null);
         }
 
-        int memFactor = mAm.mProcessStats.getMemFactorLocked();
+        synchronized (mAm.mProcessStats.mLock) {
+            final int memFactor = mAm.mProcessStats.getMemFactorLocked();
             if (r.tracker != null) {
                 r.tracker.setStarted(false, memFactor, now);
                 r.tracker.setBound(false, memFactor, now);
@@ -4312,6 +4339,7 @@ public final class ActiveServices {
                     r.tracker = null;
                 }
             }
+        }
 
         smap.ensureNotStartingBackgroundLocked(r);
     }
@@ -4440,10 +4468,12 @@ public final class ActiveServices {
                 boolean hasAutoCreate = s.hasAutoCreateConnections();
                 if (!hasAutoCreate) {
                     if (s.tracker != null) {
+                        synchronized (mAm.mProcessStats.mLock) {
                             s.tracker.setBound(false, mAm.mProcessStats.getMemFactorLocked(),
                                     SystemClock.uptimeMillis());
                         }
                     }
+                }
                 bringDownServiceIfNeededLocked(s, true, hasAutoCreate, enqueueOomAdj);
             }
         }
@@ -4533,13 +4563,15 @@ public final class ActiveServices {
 
     private void serviceProcessGoneLocked(ServiceRecord r, boolean enqueueOomAdj) {
         if (r.tracker != null) {
-            int memFactor = mAm.mProcessStats.getMemFactorLocked();
+            synchronized (mAm.mProcessStats.mLock) {
-            long now = SystemClock.uptimeMillis();
+                final int memFactor = mAm.mProcessStats.getMemFactorLocked();
+                final long now = SystemClock.uptimeMillis();
                 r.tracker.setExecuting(false, memFactor, now);
                 r.tracker.setForeground(false, memFactor, now);
                 r.tracker.setBound(false, memFactor, now);
                 r.tracker.setStarted(false, memFactor, now);
             }
+        }
         serviceDoneExecutingLocked(r, true, true, enqueueOomAdj);
     }
 
@@ -4585,6 +4617,7 @@ public final class ActiveServices {
             }
             r.executeFg = false;
             if (r.tracker != null) {
+                synchronized (mAm.mProcessStats.mLock) {
                     final int memFactor = mAm.mProcessStats.getMemFactorLocked();
                     final long now = SystemClock.uptimeMillis();
                     r.tracker.setExecuting(false, memFactor, now);
@@ -4594,6 +4627,7 @@ public final class ActiveServices {
                         r.tracker = null;
                     }
                 }
+            }
             if (finishing) {
                 if (r.app != null && !r.app.isPersistent()) {
                     stopServiceAndUpdateAllowlistManagerLocked(r);
@@ -4976,12 +5010,14 @@ public final class ActiveServices {
                     // down it.
                     sr.startRequested = false;
                     if (sr.tracker != null) {
+                        synchronized (mAm.mProcessStats.mLock) {
                             sr.tracker.setStarted(false, mAm.mProcessStats.getMemFactorLocked(),
                                     SystemClock.uptimeMillis());
                         }
                     }
                 }
             }
+        }
 
         mAm.updateOomAdjPendingTargetsLocked(OomAdjuster.OOM_ADJ_REASON_UNBIND_SERVICE);
 

services/core/java/com/android/server/am/ConnectionRecord.java

@@ -47,6 +47,7 @@ final class ConnectionRecord {
     public AssociationState.SourceState association; // Association tracking
     String stringName;              // Caching of toString.
     boolean serviceDead;            // Well is it?
+    private Object mProcStatsLock;  // Internal lock for accessing AssociationState
 
     // Please keep the following two enum list synced.
     private static final int[] BIND_ORIG_ENUMS = new int[] {
@@ -137,23 +138,29 @@ final class ConnectionRecord {
                 Slog.wtf(TAG_AM, "Inactive holder in referenced service "
                         + binding.service.shortInstanceName + ": proc=" + binding.service.app);
             } else {
+                mProcStatsLock = binding.service.app.mService.mProcessStats.mLock;
+                synchronized (mProcStatsLock) {
                     association = holder.pkg.getAssociationStateLocked(holder.state,
                             binding.service.instanceName.getClassName()).startSource(clientUid,
                             clientProcessName, clientPackageName);
-
+                }
             }
         }
     }
 
     public void trackProcState(int procState, int seq, long now) {
         if (association != null) {
+            synchronized (mProcStatsLock) {
                 association.trackProcState(procState, seq, now);
             }
         }
+    }
 
     public void stopAssociation() {
         if (association != null) {
+            synchronized (mProcStatsLock) {
                 association.stop();
+            }
             association = null;
         }
     }

services/core/java/com/android/server/am/ContentProviderConnection.java

@@ -38,6 +38,7 @@ public final class ContentProviderConnection extends Binder {
     public final String clientPackage;
     public AssociationState.SourceState association;
     public final long createTime;
+    private Object mProcStatsLock;  // Internal lock for accessing AssociationState
 
     /**
      * Internal lock that guards access to the two counters.
@@ -87,23 +88,29 @@ public final class ContentProviderConnection extends Binder {
                 Slog.wtf(TAG_AM, "Inactive holder in referenced provider "
                         + provider.name.toShortString() + ": proc=" + provider.proc);
             } else {
+                mProcStatsLock = provider.proc.mService.mProcessStats.mLock;
+                synchronized (mProcStatsLock) {
                     association = holder.pkg.getAssociationStateLocked(holder.state,
-                        provider.name.getClassName()).startSource(client.uid, client.processName,
+                            provider.name.getClassName()).startSource(client.uid,
-                        clientPackage);
+                            client.processName, clientPackage);
-
+                }
             }
         }
     }
 
     public void trackProcState(int procState, int seq, long now) {
         if (association != null) {
+            synchronized (mProcStatsLock) {
                 association.trackProcState(procState, seq, now);
             }
         }
+    }
 
     public void stopAssociation() {
         if (association != null) {
+            synchronized (mProcStatsLock) {
                 association.stop();
+            }
             association = null;
         }
     }

services/core/java/com/android/server/am/ContentProviderRecord.java

@@ -321,6 +321,7 @@ final class ContentProviderRecord implements ComponentName.WithComponentName {
         final String mOwningProcessName;
         int mAcquisitionCount;
         AssociationState.SourceState mAssociation;
+        private Object mProcStatsLock;  // Internal lock for accessing AssociationState
 
         public ExternalProcessHandle(IBinder token, int owningUid, String owningProcessName) {
             mToken = token;
@@ -353,17 +354,21 @@ final class ContentProviderRecord implements ComponentName.WithComponentName {
                     Slog.wtf(TAG_AM, "Inactive holder in referenced provider "
                             + provider.name.toShortString() + ": proc=" + provider.proc);
                 } else {
+                    mProcStatsLock = provider.proc.mService.mProcessStats.mLock;
+                    synchronized (mProcStatsLock) {
                         mAssociation = holder.pkg.getAssociationStateLocked(holder.state,
                                 provider.name.getClassName()).startSource(mOwningUid,
                                 mOwningProcessName, null);
-
+                    }
                 }
             }
         }
 
         public void stopAssociation() {
             if (mAssociation != null) {
+                synchronized (mProcStatsLock) {
                     mAssociation.stop();
+                }
                 mAssociation = null;
             }
         }

services/core/java/com/android/server/am/OomAdjuster.java

@@ -2792,8 +2792,10 @@ public class OomAdjuster {
                 state.setNotCachedSinceIdle(false);
             }
             if (!doingAll) {
+                synchronized (mService.mProcessStats.mLock) {
                     mService.setProcessTrackerStateLOSP(app,
                             mService.mProcessStats.getMemFactorLocked(), now);
+                }
             } else {
                 state.setProcStateChanged(true);
             }