Merge "Make isAppBad() lock-free" (9bace591) · Commits · e / os / android_frameworks_base

core/java/com/android/internal/app/ProcessMap.java

+3 −1

Original line number	Diff line number	Diff line
		@@ -62,4 +62,6 @@ public class ProcessMap<E> {
		public void clear() {
		mMap.clear();
		}

		public void putAll(ProcessMap<E> other) { mMap.putAll(other.mMap); }
		}

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

+93 −79

Original line number	Diff line number	Diff line
		@@ -52,7 +52,6 @@ import android.util.SparseArray;
		import android.util.TimeUtils;
		import android.util.proto.ProtoOutputStream;

		import com.android.internal.annotations.GuardedBy;
		import com.android.internal.app.ProcessMap;
		import com.android.internal.logging.MetricsLogger;
		import com.android.internal.logging.nano.MetricsProto;
		@@ -106,12 +105,18 @@ class AppErrors {
		* later restarted (hopefully due to some user action). The value is the
		* time it was added to the list.
		*
		* Access is synchronized on the container object itself, and no other
		* locks may be acquired while holding that one.
		* Read access is UNLOCKED, and must either be based on a single lookup
		* call on the current mBadProcesses instance, or a local copy of that
		* reference must be made and the local copy treated as the source of
		* truth. Mutations are performed by synchronizing on mBadProcessLock,
		* cloning the existing mBadProcesses instance, performing the mutation,
		* then changing the volatile "live" mBadProcesses reference to point to the
		* mutated version. These operations are very rare compared to lookups:
		* we intentionally trade additional cost for mutations for eliminating
		* lock operations from the simple lookup cases.
		*/
		@GuardedBy("mBadProcesses")
		private final ProcessMap<BadProcessInfo> mBadProcesses = new ProcessMap<>();

		private volatile ProcessMap<BadProcessInfo> mBadProcesses = new ProcessMap<>();
		private final Object mBadProcessLock = new Object();

		AppErrors(Context context, ActivityManagerService service, PackageWatchdog watchdog) {
		context.assertRuntimeOverlayThemable();
		@@ -128,14 +133,14 @@ class AppErrors {
		mProcessCrashTimesPersistent.clear();
		mProcessCrashShowDialogTimes.clear();
		mProcessCrashCounts.clear();
		synchronized (mBadProcesses) {
		mBadProcesses.clear();
		synchronized (mBadProcessLock) {
		mBadProcesses = new ProcessMap<>();
		}
		}

		void dumpDebug(ProtoOutputStream proto, long fieldId, String dumpPackage) {
		synchronized (mBadProcesses) {
		if (mProcessCrashTimes.getMap().isEmpty() && mBadProcesses.getMap().isEmpty()) {
		final ProcessMap<BadProcessInfo> badProcesses = mBadProcesses;
		if (mProcessCrashTimes.getMap().isEmpty() && badProcesses.getMap().isEmpty()) {
		return;
		}

		@@ -171,8 +176,8 @@ class AppErrors {

		}

		if (!mBadProcesses.getMap().isEmpty()) {
		final ArrayMap<String, SparseArray<BadProcessInfo>> pmap = mBadProcesses.getMap();
		if (!badProcesses.getMap().isEmpty()) {
		final ArrayMap<String, SparseArray<BadProcessInfo>> pmap = badProcesses.getMap();
		final int processCount = pmap.size();
		for (int ip = 0; ip < processCount; ip++) {
		final long btoken = proto.start(AppErrorsProto.BAD_PROCESSES);
		@@ -203,7 +208,6 @@ class AppErrors {

		proto.end(token);
		}
		}

		boolean dumpLocked(FileDescriptor fd, PrintWriter pw, boolean needSep, String dumpPackage) {
		final long now = SystemClock.uptimeMillis();
		@@ -267,9 +271,10 @@ class AppErrors {
		}
		}

		if (!mBadProcesses.getMap().isEmpty()) {
		final ProcessMap<BadProcessInfo> badProcesses = mBadProcesses;
		if (!badProcesses.getMap().isEmpty()) {
		boolean printed = false;
		final ArrayMap<String, SparseArray<BadProcessInfo>> pmap = mBadProcesses.getMap();
		final ArrayMap<String, SparseArray<BadProcessInfo>> pmap = badProcesses.getMap();
		final int processCount = pmap.size();
		for (int ip = 0; ip < processCount; ip++) {
		final String pname = pmap.keyAt(ip);
		@@ -322,14 +327,25 @@ class AppErrors {
		}

		boolean isBadProcess(final String processName, final int uid) {
		synchronized (mBadProcesses) {
		// NO LOCKING for the simple lookup
		return mBadProcesses.get(processName, uid) != null;
		}
		}

		void clearBadProcess(final String processName, final int uid) {
		synchronized (mBadProcesses) {
		mBadProcesses.remove(processName, uid);
		synchronized (mBadProcessLock) {
		final ProcessMap<BadProcessInfo> badProcesses = new ProcessMap<>();
		badProcesses.putAll(mBadProcesses);
		badProcesses.remove(processName, uid);
		mBadProcesses = badProcesses;
		}
		}

		void markBadProcess(final String processName, final int uid, BadProcessInfo info) {
		synchronized (mBadProcessLock) {
		final ProcessMap<BadProcessInfo> badProcesses = new ProcessMap<>();
		badProcesses.putAll(mBadProcesses);
		badProcesses.put(processName, uid, info);
		mBadProcesses = badProcesses;
		}
		}

		@@ -813,11 +829,9 @@ class AppErrors {
		app.processName);
		if (!app.isolated) {
		// XXX We don't have a way to mark isolated processes
		// as bad, since they don't have a peristent identity.
		synchronized (mBadProcesses) {
		mBadProcesses.put(app.processName, app.uid,
		// as bad, since they don't have a persistent identity.
		markBadProcess(app.processName, app.uid,
		new BadProcessInfo(now, shortMsg, longMsg, stackTrace));
		}
		mProcessCrashTimes.remove(app.processName, app.uid);
		mProcessCrashCounts.remove(app.processName, app.uid);
		}