Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c1dad7f2 authored by Lee Shombert's avatar Lee Shombert
Browse files

Update ContentProvider counters outside AMS lock

Bug: 157136568

ContentProviderConnection has a set of reference counts that are mostly
updated within blocks guarded by the ActivityManagerService (AMS) lock.
The exception is refContentProvider(), which is using the AMS lock
solely to guard the reference count updates.  This change makes ref
count updates atomic under control of the local
ContentProviderConnection.

ContentProviderConnection reference counts are now private and must be
modified through accessor methods that enforce coherence.  The total
reference count is returned.  ActivityManagerService methods always
operate on the total reference count values that result from the
method's increment or decrement operation, as follows:
* A count of 1 means the object has just been created and must be
  initialized.
* A count of 0 means the object is no longer used and should be
  reclaimed.

Test: A/B comparisons between baseline and the changed image, with the
following atests:
 * FrameworksServicesTests:UserSystemPackageInstallerTest
 * PtsChreTestCases
 * FrameworksServicesTests:UserManagerServiceCreateProfileTest
 * FrameworksServicesTests:UserManagerServiceIdRecyclingTest
 * FrameworksServicesTests:UserManagerServiceUserInfoTest
 * FrameworksServicesTests:UserLifecycleTests
 * BluetoothInstrumentationTests
 * CtsCalendarProviderTestCases
 * CtsContactsProviderTestCases
 * CtsProviderTestCases
The change introduced no failures.

Change-Id: I688573ce7aee34d98ec510f432b9eb5cc5929670
parent 44aeb8e7
Loading
Loading
Loading
Loading
+33 −75
Original line number Diff line number Diff line
@@ -6912,62 +6912,54 @@ public class ActivityManagerService extends IActivityManager.Stub
        return msg;
    }
    ContentProviderConnection incProviderCountLocked(ProcessRecord r,
    @GuardedBy("this")
    private ContentProviderConnection incProviderCountLocked(ProcessRecord r,
            final ContentProviderRecord cpr, IBinder externalProcessToken, int callingUid,
            String callingPackage, String callingTag, boolean stable) {
            String callingPackage, String callingTag, boolean stable,
            boolean updateLru, long startTime) {
        if (r != null) {
            for (int i=0; i<r.conProviders.size(); i++) {
                ContentProviderConnection conn = r.conProviders.get(i);
                if (conn.provider == cpr) {
                    if (DEBUG_PROVIDER) Slog.v(TAG_PROVIDER,
                            "Adding provider requested by "
                            + r.processName + " from process "
                            + cpr.info.processName + ": " + cpr.name.flattenToShortString()
                            + " scnt=" + conn.stableCount + " uscnt=" + conn.unstableCount);
                    if (stable) {
                        conn.stableCount++;
                        conn.numStableIncs++;
                    } else {
                        conn.unstableCount++;
                        conn.numUnstableIncs++;
                    }
                    conn.incrementCount(stable);
                    return conn;
                }
            }
            // Create a new ContentProviderConnection.  The reference count
            // is known to be 1.
            ContentProviderConnection conn = new ContentProviderConnection(cpr, r, callingPackage);
            conn.startAssociationIfNeeded();
            if (stable) {
                conn.stableCount = 1;
                conn.numStableIncs = 1;
            } else {
                conn.unstableCount = 1;
                conn.numUnstableIncs = 1;
            }
            conn.initializeCount(stable);
            cpr.connections.add(conn);
            r.conProviders.add(conn);
            startAssociationLocked(r.uid, r.processName, r.getCurProcState(),
                    cpr.uid, cpr.appInfo.longVersionCode, cpr.name, cpr.info.processName);
            if (updateLru && cpr.proc != null
                    && r != null && r.setAdj <= ProcessList.PERCEPTIBLE_LOW_APP_ADJ) {
                // If this is a perceptible app accessing the provider, make
                // sure to count it as being accessed and thus back up on
                // the LRU list.  This is good because content providers are
                // often expensive to start.  The calls to checkTime() use
                // the "getContentProviderImpl" tag here, because it's part
                // of the checktime log in getContentProviderImpl().
                checkTime(startTime, "getContentProviderImpl: before updateLruProcess");
                mProcessList.updateLruProcessLocked(cpr.proc, false, null);
                checkTime(startTime, "getContentProviderImpl: after updateLruProcess");
            }
            return conn;
        }
        cpr.addExternalProcessHandleLocked(externalProcessToken, callingUid, callingTag);
        return null;
    }
    boolean decProviderCountLocked(ContentProviderConnection conn,
    @GuardedBy("this")
    private boolean decProviderCountLocked(ContentProviderConnection conn,
            ContentProviderRecord cpr, IBinder externalProcessToken, boolean stable) {
        if (conn != null) {
            cpr = conn.provider;
            if (DEBUG_PROVIDER) Slog.v(TAG_PROVIDER,
                    "Removing provider requested by "
                    + conn.client.processName + " from process "
                    + cpr.info.processName + ": " + cpr.name.flattenToShortString()
                    + " scnt=" + conn.stableCount + " uscnt=" + conn.unstableCount);
            if (stable) {
                conn.stableCount--;
            } else {
                conn.unstableCount--;
            }
            if (conn.stableCount == 0 && conn.unstableCount == 0) {
            final int referenceCount = conn.decrementCount(stable);
            if (referenceCount == 0) {
                conn.stopAssociation();
                cpr.connections.remove(conn);
                conn.client.conProviders.remove(conn);
@@ -7165,20 +7157,8 @@ public class ActivityManagerService extends IActivityManager.Stub
                // In this case the provider instance already exists, so we can
                // return it right away.
                conn = incProviderCountLocked(r, cpr, token, callingUid, callingPackage, callingTag,
                        stable);
                if (conn != null && (conn.stableCount+conn.unstableCount) == 1) {
                    if (cpr.proc != null
                            && r != null && r.setAdj <= ProcessList.PERCEPTIBLE_LOW_APP_ADJ) {
                        // If this is a perceptible app accessing the provider,
                        // make sure to count it as being accessed and thus
                        // back up on the LRU list.  This is good because
                        // content providers are often expensive to start.
                        checkTime(startTime, "getContentProviderImpl: before updateLruProcess");
                        mProcessList.updateLruProcessLocked(cpr.proc, false, null);
                        checkTime(startTime, "getContentProviderImpl: after updateLruProcess");
                    }
                }
                conn = incProviderCountLocked(r, cpr, token, callingUid, callingPackage,
                        callingTag, stable, true, startTime);
                checkTime(startTime, "getContentProviderImpl: before updateOomAdj");
                final int verifiedAdj = cpr.proc.verifiedAdj;
@@ -7423,8 +7403,8 @@ public class ActivityManagerService extends IActivityManager.Stub
                }
                mProviderMap.putProviderByName(name, cpr);
                conn = incProviderCountLocked(r, cpr, token, callingUid, callingPackage, callingTag,
                        stable);
                conn = incProviderCountLocked(r, cpr, token, callingUid,
                        callingPackage, callingTag, stable, false, startTime);
                if (conn != null) {
                    conn.waiting = true;
                }
@@ -7760,6 +7740,7 @@ public class ActivityManagerService extends IActivityManager.Stub
        }
    }
    @Override
    public boolean refContentProvider(IBinder connection, int stable, int unstable) {
        ContentProviderConnection conn;
        try {
@@ -7774,32 +7755,9 @@ public class ActivityManagerService extends IActivityManager.Stub
            throw new NullPointerException("connection is null");
        }
        synchronized (this) {
            if (stable > 0) {
                conn.numStableIncs += stable;
            }
            stable = conn.stableCount + stable;
            if (stable < 0) {
                throw new IllegalStateException("stableCount < 0: " + stable);
            }
            if (unstable > 0) {
                conn.numUnstableIncs += unstable;
            }
            unstable = conn.unstableCount + unstable;
            if (unstable < 0) {
                throw new IllegalStateException("unstableCount < 0: " + unstable);
            }
            if ((stable+unstable) <= 0) {
                throw new IllegalStateException("ref counts can't go to zero here: stable="
                        + stable + " unstable=" + unstable);
            }
            conn.stableCount = stable;
            conn.unstableCount = unstable;
        conn.adjustCounts(stable, unstable);
        return !conn.dead;
    }
    }
    public void unstableProviderDied(IBinder connection) {
        ContentProviderConnection conn;
@@ -14732,7 +14690,7 @@ public class ActivityManagerService extends IActivityManager.Stub
            }
            ProcessRecord capp = conn.client;
            conn.dead = true;
            if (conn.stableCount > 0) {
            if (conn.stableCount() > 0) {
                if (!capp.isPersistent() && capp.thread != null
                        && capp.pid != 0
                        && capp.pid != MY_PID) {
+139 −12
Original line number Diff line number Diff line
@@ -21,9 +21,11 @@ import android.os.SystemClock;
import android.util.Slog;
import android.util.TimeUtils;

import com.android.internal.annotations.GuardedBy;
import com.android.internal.app.procstats.AssociationState;
import com.android.internal.app.procstats.ProcessStats;

import static com.android.server.am.ActivityManagerDebugConfig.DEBUG_PROVIDER;
import static com.android.server.am.ActivityManagerDebugConfig.TAG_AM;

/**
@@ -35,8 +37,15 @@ public final class ContentProviderConnection extends Binder {
    public final String clientPackage;
    public AssociationState.SourceState association;
    public final long createTime;
    public int stableCount;
    public int unstableCount;

    /**
     * Internal lock that guards access to the two counters.
     */
    private final Object mLock = new Object();
    @GuardedBy("mLock")
    private int mStableCount;
    @GuardedBy("mLock")
    private int mUnstableCount;
    // The client of this connection is currently waiting for the provider to appear.
    // Protected by the provider lock.
    public boolean waiting;
@@ -44,8 +53,8 @@ public final class ContentProviderConnection extends Binder {
    public boolean dead;

    // For debugging.
    public int numStableIncs;
    public int numUnstableIncs;
    private int mNumStableIncs;
    private int mNumUnstableIncs;

    public ContentProviderConnection(ContentProviderRecord _provider, ProcessRecord _client,
            String _clientPackage) {
@@ -120,14 +129,16 @@ public final class ContentProviderConnection extends Binder {

    public void toClientString(StringBuilder sb) {
        sb.append(client.toShortString());
        synchronized (mLock) {
            sb.append(" s");
        sb.append(stableCount);
            sb.append(mStableCount);
            sb.append("/");
        sb.append(numStableIncs);
            sb.append(mNumStableIncs);
            sb.append(" u");
        sb.append(unstableCount);
            sb.append(mUnstableCount);
            sb.append("/");
        sb.append(numUnstableIncs);
            sb.append(mNumUnstableIncs);
        }
        if (waiting) {
            sb.append(" WAITING");
        }
@@ -138,4 +149,120 @@ public final class ContentProviderConnection extends Binder {
        sb.append(" ");
        TimeUtils.formatDuration(nowReal-createTime, sb);
    }

    /**
     * Initializes the reference counts.  Either the stable or unstable count
     * is set to 1; the other reference count is set to zero.
     */
    public void initializeCount(boolean stable) {
        synchronized (mLock) {
            if (stable) {
                mStableCount = 1;
                mNumStableIncs = 1;
                mUnstableCount = 0;
                mNumUnstableIncs = 0;
            } else {
                mStableCount = 0;
                mNumStableIncs = 0;
                mUnstableCount = 1;
                mNumUnstableIncs = 1;
            }
        }
    }

    /**
     * Increments the stable or unstable reference count and return the total
     * number of references.
     */
    public int incrementCount(boolean stable) {
        synchronized (mLock) {
            if (DEBUG_PROVIDER) {
                final ContentProviderRecord cpr = provider;
                Slog.v(TAG_AM,
                       "Adding provider requested by "
                       + client.processName + " from process "
                       + cpr.info.processName + ": " + cpr.name.flattenToShortString()
                       + " scnt=" + mStableCount + " uscnt=" + mUnstableCount);
            }
            if (stable) {
                mStableCount++;
                mNumStableIncs++;
            } else {
                mUnstableCount++;
                mNumUnstableIncs++;
            }
            return mStableCount + mUnstableCount;
        }
    }

    /**
     * Decrements either the stable or unstable count and return the total
     * number of references.
     */
    public int decrementCount(boolean stable) {
        synchronized (mLock) {
            if (DEBUG_PROVIDER) {
                final ContentProviderRecord cpr = provider;
                Slog.v(TAG_AM,
                       "Removing provider requested by "
                       + client.processName + " from process "
                       + cpr.info.processName + ": " + cpr.name.flattenToShortString()
                       + " scnt=" + mStableCount + " uscnt=" + mUnstableCount);
            }
            if (stable) {
                mStableCount--;
            } else {
                mUnstableCount--;
            }
            return mStableCount + mUnstableCount;
        }
    }

    /**
     * Adjusts the reference counts up or down (the inputs may be positive,
     * zero, or negative.  This method does not return a total count because
     * a return is not needed for the current use case.
    */
    public void adjustCounts(int stableIncrement, int unstableIncrement) {
        synchronized (mLock) {
            if (stableIncrement > 0) {
                mNumStableIncs += stableIncrement;
            }
            final int stable = mStableCount + stableIncrement;
            if (stable < 0) {
                throw new IllegalStateException("stableCount < 0: " + stable);
            }
            if (unstableIncrement > 0) {
                mNumUnstableIncs += unstableIncrement;
            }
            final int unstable = mUnstableCount + unstableIncrement;
            if (unstable < 0) {
                throw new IllegalStateException("unstableCount < 0: " + unstable);
            }
            if ((stable + unstable) <= 0) {
                throw new IllegalStateException("ref counts can't go to zero here: stable="
                                                + stable + " unstable=" + unstable);
            }
            mStableCount = stable;
            mUnstableCount = unstable;
        }
    }

    /**
     * Returns the number of stable references.
     */
    public int stableCount() {
        synchronized (mLock) {
            return mStableCount;
        }
    }

    /**
     * Returns the number of unstable references.
     */
    public int unstableCount() {
        synchronized (mLock) {
            return mUnstableCount;
        }
    }
}