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

5 files changed, 139 insertions, 82 deletions

diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java
index 45e6f1ec4bf8..f41036cde433 100644
--- a/services/core/java/com/android/server/am/ActiveServices.java
+++ b/services/core/java/com/android/server/am/ActiveServices.java
@@ -807,10 +807,12 @@ public final class ActiveServices {
 
         if (fgRequired) {
             // We are now effectively running a foreground service.
-            ServiceState stracker = r.getTracker();
-            if (stracker != null) {
-                stracker.setForeground(true, mAm.mProcessStats.getMemFactorLocked(),
-                        r.lastActivity);
+            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,
@@ -1082,9 +1084,11 @@ public final class ActiveServices {
 
     ComponentName startServiceInnerLocked(ServiceMap smap, Intent service, ServiceRecord r,
             boolean callerFg, boolean addToStarting) throws TransactionTooLargeException {
-        ServiceState stracker = r.getTracker();
-        if (stracker != null) {
-            stracker.setStarted(true, mAm.mProcessStats.getMemFactorLocked(), r.lastActivity);
+        synchronized (mAm.mProcessStats.mLock) {
+            final ServiceState stracker = r.getTracker();
+            if (stracker != null) {
+                stracker.setStarted(true, mAm.mProcessStats.getMemFactorLocked(), r.lastActivity);
+            }
         }
         r.callStart = false;
 
@@ -1144,8 +1148,10 @@ public final class ActiveServices {
         mAm.mBatteryStatsService.noteServiceStopRunning(uid, packageName, serviceName);
         service.startRequested = false;
         if (service.tracker != null) {
-            service.tracker.setStarted(false, mAm.mProcessStats.getMemFactorLocked(),
-                    SystemClock.uptimeMillis());
+            synchronized (mAm.mProcessStats.mLock) {
+                service.tracker.setStarted(false, mAm.mProcessStats.getMemFactorLocked(),
+                        SystemClock.uptimeMillis());
+            }
         }
         service.callStart = false;
 
@@ -1320,8 +1326,10 @@ public final class ActiveServices {
             mAm.mBatteryStatsService.noteServiceStopRunning(uid, packageName, serviceName);
             r.startRequested = false;
             if (r.tracker != null) {
-                r.tracker.setStarted(false, mAm.mProcessStats.getMemFactorLocked(),
-                        SystemClock.uptimeMillis());
+                synchronized (mAm.mProcessStats.mLock) {
+                    r.tracker.setStarted(false, mAm.mProcessStats.getMemFactorLocked(),
+                            SystemClock.uptimeMillis());
+                }
             }
             r.callStart = false;
             final long origId = Binder.clearCallingIdentity();
@@ -1877,10 +1885,12 @@ public final class ActiveServices {
                         r.mStartForegroundCount++;
                         r.mFgsEnterTime = SystemClock.uptimeMillis();
                         if (!stopProcStatsOp) {
-                            ServiceState stracker = r.getTracker();
-                            if (stracker != null) {
-                                stracker.setForeground(true,
-                                        mAm.mProcessStats.getMemFactorLocked(), r.lastActivity);
+                            synchronized (mAm.mProcessStats.mLock) {
+                                final ServiceState stracker = r.getTracker();
+                                if (stracker != null) {
+                                    stracker.setForeground(true,
+                                            mAm.mProcessStats.getMemFactorLocked(), r.lastActivity);
+                                }
                             }
                         } else {
                             stopProcStatsOp = false;
@@ -1915,10 +1925,12 @@ 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();
-                    if (stracker != null) {
-                        stracker.setForeground(false, mAm.mProcessStats.getMemFactorLocked(),
-                                SystemClock.uptimeMillis());
+                    synchronized (mAm.mProcessStats.mLock) {
+                        final ServiceState stracker = r.getTracker();
+                        if (stracker != null) {
+                            stracker.setForeground(false, mAm.mProcessStats.getMemFactorLocked(),
+                                    SystemClock.uptimeMillis());
+                        }
                     }
                 }
                 if (alreadyStartedOp) {
@@ -1960,10 +1972,12 @@ public final class ActiveServices {
 
                 r.isForeground = false;
                 r.mFgsExitTime = SystemClock.uptimeMillis();
-                ServiceState stracker = r.getTracker();
-                if (stracker != null) {
-                    stracker.setForeground(false, mAm.mProcessStats.getMemFactorLocked(),
-                            SystemClock.uptimeMillis());
+                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),
@@ -2738,10 +2752,12 @@ public final class ActiveServices {
                 s.lastActivity = SystemClock.uptimeMillis();
                 if (!s.hasAutoCreateConnections()) {
                     // This is the first binding, let the tracker know.
-                    ServiceState stracker = s.getTracker();
-                    if (stracker != null) {
-                        stracker.setBound(true, mAm.mProcessStats.getMemFactorLocked(),
-                                s.lastActivity);
+                    synchronized (mAm.mProcessStats.mLock) {
+                        final ServiceState stracker = s.getTracker();
+                        if (stracker != null) {
+                            stracker.setBound(true, mAm.mProcessStats.getMemFactorLocked(),
+                                    s.lastActivity);
+                        }
                     }
                 }
             }
@@ -3378,9 +3394,11 @@ public final class ActiveServices {
         ProcessServiceRecord psr;
         if (r.executeNesting == 0) {
             r.executeFg = fg;
-            ServiceState stracker = r.getTracker();
-            if (stracker != null) {
-                stracker.setExecuting(true, mAm.mProcessStats.getMemFactorLocked(), now);
+            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;
@@ -3575,7 +3593,9 @@ public final class ActiveServices {
         if (!mRestartingServices.contains(r)) {
             r.createdFromFg = false;
             mRestartingServices.add(r);
-            r.makeRestarting(mAm.mProcessStats.getMemFactorLocked(), now);
+            synchronized (mAm.mProcessStats.mLock) {
+                r.makeRestarting(mAm.mProcessStats.getMemFactorLocked(), now);
+            }
         }
 
         cancelForegroundNotificationLocked(r);
@@ -3653,8 +3673,10 @@ public final class ActiveServices {
                 }
             }
             if (!stillTracking) {
-                r.restartTracker.setRestarting(false, mAm.mProcessStats.getMemFactorLocked(),
-                        SystemClock.uptimeMillis());
+                synchronized (mAm.mProcessStats.mLock) {
+                    r.restartTracker.setRestarting(false, mAm.mProcessStats.getMemFactorLocked(),
+                            SystemClock.uptimeMillis());
+                }
                 r.restartTracker = null;
             }
         }
@@ -4174,9 +4196,11 @@ public final class ActiveServices {
                     + r);
             r.fgRequired = false;
             r.fgWaiting = false;
-            ServiceState stracker = r.getTracker();
-            if (stracker != null) {
-                stracker.setForeground(false, mAm.mProcessStats.getMemFactorLocked(), now);
+            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);
@@ -4233,9 +4257,11 @@ public final class ActiveServices {
         cancelForegroundNotificationLocked(r);
         if (r.isForeground) {
             decActiveForegroundAppLocked(smap, r);
-            ServiceState stracker = r.getTracker();
-            if (stracker != null) {
-                stracker.setForeground(false, mAm.mProcessStats.getMemFactorLocked(), now);
+            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),
@@ -4303,13 +4329,15 @@ public final class ActiveServices {
            ((ServiceRestarter)r.restarter).setService(null);
         }
 
-        int memFactor = mAm.mProcessStats.getMemFactorLocked();
-        if (r.tracker != null) {
-            r.tracker.setStarted(false, memFactor, now);
-            r.tracker.setBound(false, memFactor, now);
-            if (r.executeNesting == 0) {
-                r.tracker.clearCurrentOwner(r, false);
-                r.tracker = null;
+        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);
+                if (r.executeNesting == 0) {
+                    r.tracker.clearCurrentOwner(r, false);
+                    r.tracker = null;
+                }
             }
         }
 
@@ -4440,8 +4468,10 @@ public final class ActiveServices {
                 boolean hasAutoCreate = s.hasAutoCreateConnections();
                 if (!hasAutoCreate) {
                     if (s.tracker != null) {
-                        s.tracker.setBound(false, mAm.mProcessStats.getMemFactorLocked(),
-                                SystemClock.uptimeMillis());
+                        synchronized (mAm.mProcessStats.mLock) {
+                            s.tracker.setBound(false, mAm.mProcessStats.getMemFactorLocked(),
+                                    SystemClock.uptimeMillis());
+                        }
                     }
                 }
                 bringDownServiceIfNeededLocked(s, true, hasAutoCreate, enqueueOomAdj);
@@ -4533,12 +4563,14 @@ public final class ActiveServices {
 
     private void serviceProcessGoneLocked(ServiceRecord r, boolean enqueueOomAdj) {
         if (r.tracker != null) {
-            int memFactor = mAm.mProcessStats.getMemFactorLocked();
-            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);
+            synchronized (mAm.mProcessStats.mLock) {
+                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,13 +4617,15 @@ public final class ActiveServices {
             }
             r.executeFg = false;
             if (r.tracker != null) {
-                final int memFactor = mAm.mProcessStats.getMemFactorLocked();
-                final long now = SystemClock.uptimeMillis();
-                r.tracker.setExecuting(false, memFactor, now);
-                r.tracker.setForeground(false, memFactor, now);
-                if (finishing) {
-                    r.tracker.clearCurrentOwner(r, false);
-                    r.tracker = null;
+                synchronized (mAm.mProcessStats.mLock) {
+                    final int memFactor = mAm.mProcessStats.getMemFactorLocked();
+                    final long now = SystemClock.uptimeMillis();
+                    r.tracker.setExecuting(false, memFactor, now);
+                    r.tracker.setForeground(false, memFactor, now);
+                    if (finishing) {
+                        r.tracker.clearCurrentOwner(r, false);
+                        r.tracker = null;
+                    }
                 }
             }
             if (finishing) {
@@ -4976,8 +5010,10 @@ public final class ActiveServices {
                     // down it.
                     sr.startRequested = false;
                     if (sr.tracker != null) {
-                        sr.tracker.setStarted(false, mAm.mProcessStats.getMemFactorLocked(),
-                                SystemClock.uptimeMillis());
+                        synchronized (mAm.mProcessStats.mLock) {
+                            sr.tracker.setStarted(false, mAm.mProcessStats.getMemFactorLocked(),
+                                    SystemClock.uptimeMillis());
+                        }
                     }
                 }
             }
diff --git a/services/core/java/com/android/server/am/ConnectionRecord.java b/services/core/java/com/android/server/am/ConnectionRecord.java
index e6cd509f50dc..916127126117 100644
--- a/services/core/java/com/android/server/am/ConnectionRecord.java
+++ b/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 {
-                association = holder.pkg.getAssociationStateLocked(holder.state,
-                        binding.service.instanceName.getClassName()).startSource(clientUid,
-                        clientProcessName, clientPackageName);
-
+                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) {
-            association.trackProcState(procState, seq, now);
+            synchronized (mProcStatsLock) {
+                association.trackProcState(procState, seq, now);
+            }
         }
     }
 
     public void stopAssociation() {
         if (association != null) {
-            association.stop();
+            synchronized (mProcStatsLock) {
+                association.stop();
+            }
             association = null;
         }
     }
diff --git a/services/core/java/com/android/server/am/ContentProviderConnection.java b/services/core/java/com/android/server/am/ContentProviderConnection.java
index 3bc4fcf08921..3b9d47d509ad 100644
--- a/services/core/java/com/android/server/am/ContentProviderConnection.java
+++ b/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 {
-                association = holder.pkg.getAssociationStateLocked(holder.state,
-                        provider.name.getClassName()).startSource(client.uid, client.processName,
-                        clientPackage);
-
+                mProcStatsLock = provider.proc.mService.mProcessStats.mLock;
+                synchronized (mProcStatsLock) {
+                    association = holder.pkg.getAssociationStateLocked(holder.state,
+                            provider.name.getClassName()).startSource(client.uid,
+                            client.processName, clientPackage);
+                }
             }
         }
     }
 
     public void trackProcState(int procState, int seq, long now) {
         if (association != null) {
-            association.trackProcState(procState, seq, now);
+            synchronized (mProcStatsLock) {
+                association.trackProcState(procState, seq, now);
+            }
         }
     }
 
     public void stopAssociation() {
         if (association != null) {
-            association.stop();
+            synchronized (mProcStatsLock) {
+                association.stop();
+            }
             association = null;
         }
     }
diff --git a/services/core/java/com/android/server/am/ContentProviderRecord.java b/services/core/java/com/android/server/am/ContentProviderRecord.java
index 5fd15db79d66..75f31c0ac29f 100644
--- a/services/core/java/com/android/server/am/ContentProviderRecord.java
+++ b/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 {
-                    mAssociation = holder.pkg.getAssociationStateLocked(holder.state,
-                            provider.name.getClassName()).startSource(mOwningUid,
-                            mOwningProcessName, null);
-
+                    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) {
-                mAssociation.stop();
+                synchronized (mProcStatsLock) {
+                    mAssociation.stop();
+                }
                 mAssociation = null;
             }
         }
diff --git a/services/core/java/com/android/server/am/OomAdjuster.java b/services/core/java/com/android/server/am/OomAdjuster.java
index d6bf8dbe0a6a..5c19ceb7067a 100644
--- a/services/core/java/com/android/server/am/OomAdjuster.java
+++ b/services/core/java/com/android/server/am/OomAdjuster.java
@@ -2792,8 +2792,10 @@ public class OomAdjuster {
                 state.setNotCachedSinceIdle(false);
             }
             if (!doingAll) {
-                mService.setProcessTrackerStateLOSP(app,
-                        mService.mProcessStats.getMemFactorLocked(), now);
+                synchronized (mService.mProcessStats.mLock) {
+                    mService.setProcessTrackerStateLOSP(app,
+                            mService.mProcessStats.getMemFactorLocked(), now);
+                }
             } else {
                 state.setProcStateChanged(true);
             }