From dd4751d2b3ac36cf38b1edf43d6cb2f259aeaaeb Mon Sep 17 00:00:00 2001 From: Stanislav Zholnin Date: Thu, 4 Mar 2021 00:37:33 +0000 Subject: [PATCH] Remove references to Discrete Register from AppOpsService. Also record DiscreteOp when AppOp is started, later update it with proper duration when it is finished. Previously we only recorded it when it is finished. Test: In separate CL Change-Id: Ieada561de58a6fe34911bba21f770dcb2fb87f12 --- .../android/server/appop/AppOpsService.java | 21 ++++++---------- .../server/appop/DiscreteRegistry.java | 6 +++++ .../server/appop/HistoricalRegistry.java | 25 +++++++++++++++---- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java index 11125dd556658..4a12ff6932dee 100644 --- a/services/core/java/com/android/server/appop/AppOpsService.java +++ b/services/core/java/com/android/server/appop/AppOpsService.java @@ -848,10 +848,7 @@ public class AppOpsService extends IAppOpsService.Stub { proxyAttributionTag, uidState, flags); mHistoricalRegistry.incrementOpAccessedCount(parent.op, parent.uid, parent.packageName, - tag, uidState, flags); - - mHistoricalRegistry.mDiscreteRegistry.recordDiscreteAccess(parent.uid, - parent.packageName, parent.op, tag, flags, uidState, accessTime, -1); + tag, uidState, flags, accessTime); } /** @@ -955,9 +952,10 @@ public class AppOpsService extends IAppOpsService.Stub { mInProgressEvents = new ArrayMap<>(1); } + long startTime = System.currentTimeMillis(); InProgressStartOpEvent event = mInProgressEvents.get(clientId); if (event == null) { - event = mInProgressStartOpEventPool.acquire(System.currentTimeMillis(), + event = mInProgressStartOpEventPool.acquire(startTime, SystemClock.elapsedRealtime(), clientId, PooledLambda.obtainRunnable(AppOpsService::onClientDeath, this, clientId), proxyUid, proxyPackageName, proxyAttributionTag, uidState, flags); @@ -971,7 +969,7 @@ public class AppOpsService extends IAppOpsService.Stub { event.numUnfinishedStarts++; mHistoricalRegistry.incrementOpAccessedCount(parent.op, parent.uid, parent.packageName, - tag, uidState, flags); + tag, uidState, flags, startTime); } /** @@ -1017,11 +1015,7 @@ public class AppOpsService extends IAppOpsService.Stub { mHistoricalRegistry.increaseOpAccessDuration(parent.op, parent.uid, parent.packageName, tag, event.getUidState(), - event.getFlags(), finishedEvent.getDuration()); - - mHistoricalRegistry.mDiscreteRegistry.recordDiscreteAccess(parent.uid, - parent.packageName, parent.op, tag, event.getFlags(), event.getUidState(), - event.getStartTime(), accessDurationMillis); + event.getFlags(), finishedEvent.getNoteTime(), finishedEvent.getDuration()); mInProgressStartOpEventPool.release(event); @@ -4769,7 +4763,7 @@ public class AppOpsService extends IAppOpsService.Stub { mFile.failWrite(stream); } } - mHistoricalRegistry.mDiscreteRegistry.writeAndClearAccessHistory(); + mHistoricalRegistry.writeAndClearDiscreteHistory(); } static class Shell extends ShellCommand { @@ -6125,8 +6119,7 @@ public class AppOpsService extends IAppOpsService.Stub { mContext.enforceCallingOrSelfPermission(android.Manifest.permission.MANAGE_APPOPS, "clearHistory"); // Must not hold the appops lock - mHistoricalRegistry.clearHistory(); - mHistoricalRegistry.mDiscreteRegistry.clearHistory(); + mHistoricalRegistry.clearAllHistory(); } @Override diff --git a/services/core/java/com/android/server/appop/DiscreteRegistry.java b/services/core/java/com/android/server/appop/DiscreteRegistry.java index 76990453ee03c..a99d90883f876 100644 --- a/services/core/java/com/android/server/appop/DiscreteRegistry.java +++ b/services/core/java/com/android/server/appop/DiscreteRegistry.java @@ -495,6 +495,12 @@ final class DiscreteRegistry { int nAttributedOps = attributedOps.size(); for (int i = nAttributedOps - 1; i >= 0; i--) { DiscreteOpEvent previousOp = attributedOps.get(i); + if (i == nAttributedOps - 1 && previousOp.mNoteTime == accessTime + && accessDuration > -1) { + // existing event with updated duration + attributedOps.remove(i); + break; + } if (previousOp.mNoteTime < accessTime) { break; } diff --git a/services/core/java/com/android/server/appop/HistoricalRegistry.java b/services/core/java/com/android/server/appop/HistoricalRegistry.java index 1c43fedd31121..0fcf5ee35e11a 100644 --- a/services/core/java/com/android/server/appop/HistoricalRegistry.java +++ b/services/core/java/com/android/server/appop/HistoricalRegistry.java @@ -138,7 +138,7 @@ final class HistoricalRegistry { private static final String PARAMETER_ASSIGNMENT = "="; private static final String PROPERTY_PERMISSIONS_HUB_ENABLED = "permissions_hub_enabled"; - volatile @NonNull DiscreteRegistry mDiscreteRegistry; + private volatile @NonNull DiscreteRegistry mDiscreteRegistry; @GuardedBy("mLock") private @NonNull LinkedList mPendingWrites = new LinkedList<>(); @@ -477,7 +477,8 @@ final class HistoricalRegistry { } void incrementOpAccessedCount(int op, int uid, @NonNull String packageName, - @Nullable String attributionTag, @UidState int uidState, @OpFlags int flags) { + @Nullable String attributionTag, @UidState int uidState, @OpFlags int flags, + long accessTime) { synchronized (mInMemoryLock) { if (mMode == AppOpsManager.HISTORICAL_MODE_ENABLED_ACTIVE) { if (!isPersistenceInitializedMLocked()) { @@ -487,6 +488,9 @@ final class HistoricalRegistry { getUpdatedPendingHistoricalOpsMLocked( System.currentTimeMillis()).increaseAccessCount(op, uid, packageName, attributionTag, uidState, flags, 1); + + mDiscreteRegistry.recordDiscreteAccess(uid, packageName, op, attributionTag, + flags, uidState, accessTime, -1); } } } @@ -508,7 +512,7 @@ final class HistoricalRegistry { void increaseOpAccessDuration(int op, int uid, @NonNull String packageName, @Nullable String attributionTag, @UidState int uidState, @OpFlags int flags, - long increment) { + long eventStartTime, long increment) { synchronized (mInMemoryLock) { if (mMode == AppOpsManager.HISTORICAL_MODE_ENABLED_ACTIVE) { if (!isPersistenceInitializedMLocked()) { @@ -518,6 +522,8 @@ final class HistoricalRegistry { getUpdatedPendingHistoricalOpsMLocked( System.currentTimeMillis()).increaseAccessDuration(op, uid, packageName, attributionTag, uidState, flags, increment); + mDiscreteRegistry.recordDiscreteAccess(uid, packageName, op, attributionTag, + flags, uidState, increment, eventStartTime); } } } @@ -562,7 +568,7 @@ final class HistoricalRegistry { return; } final List history = mPersistence.readHistoryDLocked(); - clearHistory(); + clearHistoricalRegistry(); if (history != null) { final int historySize = history.size(); for (int i = 0; i < historySize; i++) { @@ -631,7 +637,16 @@ final class HistoricalRegistry { } } - void clearHistory() { + void writeAndClearDiscreteHistory() { + mDiscreteRegistry.writeAndClearAccessHistory(); + } + + void clearAllHistory() { + clearHistoricalRegistry(); + mDiscreteRegistry.clearHistory(); + } + + void clearHistoricalRegistry() { synchronized (mOnDiskLock) { synchronized (mInMemoryLock) { if (!isPersistenceInitializedMLocked()) {