From b29051d45ca53c89a54395d58c02f92e6cc45dd4 Mon Sep 17 00:00:00 2001 From: mrulhania Date: Tue, 4 Feb 2025 15:37:22 -0800 Subject: Use IoThread handler for AppOpsService background/IO work AppOpsService has been using AMS handler so far for most of its work, AMS handler is passed in AppOpsService constructor. We shouldn't be using AMS handler for app ops disk io, IO handler is now used for disk read/writes. Bug: 394380603 Test: presbumit Test: manual tested the device by visiting privacy dashboard and permission timeline page FLAG: EXEMPT bug fix Change-Id: I2933e80a3b95bd34eff34abc1b51dd50ec43ce81 --- core/java/android/permission/flags.aconfig | 8 ++++++ .../com/android/server/appop/AppOpsService.java | 29 ++++++++++++++++------ 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/core/java/android/permission/flags.aconfig b/core/java/android/permission/flags.aconfig index a480a3b013bb..daa5584462ba 100644 --- a/core/java/android/permission/flags.aconfig +++ b/core/java/android/permission/flags.aconfig @@ -499,3 +499,11 @@ flag { description: "Collect sqlite performance metrics for discrete ops." bug: "377584611" } + +flag { + name: "app_ops_service_handler_fix" + is_fixed_read_only: true + namespace: "permissions" + description: "Use IoThread handler for AppOpsService background/IO work." + bug: "394380603" +} diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java index 32c4e9b1727e..2a9762caaf79 100644 --- a/services/core/java/com/android/server/appop/AppOpsService.java +++ b/services/core/java/com/android/server/appop/AppOpsService.java @@ -72,6 +72,7 @@ import static android.content.Intent.EXTRA_REPLACING; import static android.content.pm.PermissionInfo.PROTECTION_DANGEROUS; import static android.content.pm.PermissionInfo.PROTECTION_FLAG_APPOP; import static android.os.Flags.binderFrozenStateChangeCallback; +import static android.permission.flags.Flags.appOpsServiceHandlerFix; import static android.permission.flags.Flags.checkOpValidatePackage; import static android.permission.flags.Flags.deviceAwareAppOpNewSchemaEnabled; import static android.permission.flags.Flags.useFrozenAwareRemoteCallbackList; @@ -174,6 +175,7 @@ import com.android.internal.util.XmlUtils; import com.android.internal.util.function.pooled.PooledLambda; import com.android.modules.utils.TypedXmlPullParser; import com.android.modules.utils.TypedXmlSerializer; +import com.android.server.IoThread; import com.android.server.LocalManagerRegistry; import com.android.server.LocalServices; import com.android.server.LockGuard; @@ -277,6 +279,7 @@ public class AppOpsService extends IAppOpsService.Stub { final AtomicFile mStorageFile; final AtomicFile mRecentAccessesFile; private final @Nullable File mNoteOpCallerStacktracesFile; + /* AMS handler, this shouldn't be used for IO */ final Handler mHandler; private final AppOpsRecentAccessPersistence mRecentAccessPersistence; @@ -1411,7 +1414,7 @@ public class AppOpsService extends IAppOpsService.Stub { @GuardedBy("this") private void packageRemovedLocked(int uid, String packageName) { - mHandler.post(PooledLambda.obtainRunnable(HistoricalRegistry::clearHistory, + getIoHandler().post(PooledLambda.obtainRunnable(HistoricalRegistry::clearHistory, mHistoricalRegistry, uid, packageName)); UidState uidState = mUidStates.get(uid); @@ -1693,7 +1696,7 @@ public class AppOpsService extends IAppOpsService.Stub { if (mWriteScheduled) { mWriteScheduled = false; mFastWriteScheduled = false; - mHandler.removeCallbacks(mWriteRunner); + getIoHandler().removeCallbacks(mWriteRunner); doWrite = true; } } @@ -1979,7 +1982,7 @@ public class AppOpsService extends IAppOpsService.Stub { new String[attributionChainExemptPackages.size()]) : null; // Must not hold the appops lock - mHandler.post(PooledLambda.obtainRunnable(HistoricalRegistry::getHistoricalOps, + getIoHandler().post(PooledLambda.obtainRunnable(HistoricalRegistry::getHistoricalOps, mHistoricalRegistry, uid, packageName, attributionTag, opNamesArray, dataType, filter, beginTimeMillis, endTimeMillis, flags, chainExemptPkgArray, callback).recycleOnUse()); @@ -2010,7 +2013,8 @@ public class AppOpsService extends IAppOpsService.Stub { new String[attributionChainExemptPackages.size()]) : null; // Must not hold the appops lock - mHandler.post(PooledLambda.obtainRunnable(HistoricalRegistry::getHistoricalOpsFromDiskRaw, + getIoHandler().post(PooledLambda.obtainRunnable( + HistoricalRegistry::getHistoricalOpsFromDiskRaw, mHistoricalRegistry, uid, packageName, attributionTag, opNamesArray, dataType, filter, beginTimeMillis, endTimeMillis, flags, chainExemptPkgArray, callback).recycleOnUse()); @@ -5074,7 +5078,7 @@ public class AppOpsService extends IAppOpsService.Stub { private void scheduleWriteLocked() { if (!mWriteScheduled) { mWriteScheduled = true; - mHandler.postDelayed(mWriteRunner, WRITE_DELAY); + getIoHandler().postDelayed(mWriteRunner, WRITE_DELAY); } } @@ -5082,8 +5086,8 @@ public class AppOpsService extends IAppOpsService.Stub { if (!mFastWriteScheduled) { mWriteScheduled = true; mFastWriteScheduled = true; - mHandler.removeCallbacks(mWriteRunner); - mHandler.postDelayed(mWriteRunner, 10*1000); + getIoHandler().removeCallbacks(mWriteRunner); + getIoHandler().postDelayed(mWriteRunner, 10 * 1000); } } @@ -5957,7 +5961,8 @@ public class AppOpsService extends IAppOpsService.Stub { final long token = Binder.clearCallingIdentity(); try { synchronized (shell.mInternal) { - shell.mInternal.mHandler.removeCallbacks(shell.mInternal.mWriteRunner); + shell.mInternal.getIoHandler().removeCallbacks( + shell.mInternal.mWriteRunner); } shell.mInternal.writeRecentAccesses(); shell.mInternal.mAppOpsCheckingService.writeState(); @@ -7884,4 +7889,12 @@ public class AppOpsService extends IAppOpsService.Stub { return null; } } + + private Handler getIoHandler() { + if (appOpsServiceHandlerFix()) { + return IoThread.getHandler(); + } else { + return mHandler; + } + } } -- cgit v1.2.3-59-g8ed1b