Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Avoid alarm lock when calling outside

Browse Source 
Specifically need to avoid creating a cyclical locking pattern with
device idle controller.

Test: builds, boots, existing tests pass.
m RUN_ERROR_PRONE=true services
atest FrameworksMockingServicesTests:com.android.server.alarm

Bug: 228971904
Change-Id: I93109f820690baa1e51f47fc3eec0e39a71ef08e
This commit is contained in:
Suprabh Shukla
2022-04-26 17:44:18 -07:00
parent 965e705c7a
commit 40d36f75bb
2 changed files with 24 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java
View File

@@ -2637,15 +2637,18 @@ public class AlarmManagerService extends SystemService {
* Returns true if the given uid can set window to be as small as it wants.
*/
boolean isExemptFromMinWindowRestrictions(int uid) {
return isExemptFromExactAlarmPermission(uid);
return isExemptFromExactAlarmPermissionNoLock(uid);
}
/**
* Returns true if the given uid does not require SCHEDULE_EXACT_ALARM to set exact,
* allow-while-idle alarms.
* Note: It is ok to call this method without the lock {@link #mLock} held.
* <b> Note: This should not be called with {@link #mLock} held.</b>
*/
boolean isExemptFromExactAlarmPermission(int uid) {
boolean isExemptFromExactAlarmPermissionNoLock(int uid) {
if (Build.IS_DEBUGGABLE && Thread.holdsLock(mLock)) {
Slog.wtfStack(TAG, "Alarm lock held while calling into DeviceIdleController");
}
return (UserHandle.isSameApp(mSystemUiUid, uid)
|| UserHandle.isCore(uid)
|| mLocalDeviceIdleController == null
@@ -2747,7 +2750,7 @@ public class AlarmManagerService extends SystemService {
}
if (needsPermission && !hasScheduleExactAlarmInternal(callingPackage, callingUid)
&& !hasUseExactAlarmInternal(callingPackage, callingUid)) {
if (!isExemptFromExactAlarmPermission(callingUid)) {
if (!isExemptFromExactAlarmPermissionNoLock(callingUid)) {
final String errorMessage = "Caller " + callingPackage + " needs to hold "
+ Manifest.permission.SCHEDULE_EXACT_ALARM + " to set "
+ "exact alarms.";
@@ -2810,7 +2813,7 @@ public class AlarmManagerService extends SystemService {
if (!isExactAlarmChangeEnabled(packageName, userId)) {
return true;
}
return isExemptFromExactAlarmPermission(packageUid)
return isExemptFromExactAlarmPermissionNoLock(packageUid)
|| hasScheduleExactAlarmInternal(packageName, packageUid)
|| hasUseExactAlarmInternal(packageName, packageUid);
}
@@ -3862,10 +3865,7 @@ public class AlarmManagerService extends SystemService {
// added: true => package was added to the deny list
// added: false => package was removed from the deny list
if (added) {
synchronized (mLock) {
removeExactAlarmsOnPermissionRevokedLocked(uid,
changedPackage, /*killUid = */ true);
}
removeExactAlarmsOnPermissionRevoked(uid, changedPackage, /*killUid = */ true);
} else {
sendScheduleExactAlarmPermissionStateChangedBroadcast(changedPackage, userId);
}
@@ -3880,9 +3880,8 @@ public class AlarmManagerService extends SystemService {
*
* This is not expected to get called frequently.
*/
@GuardedBy("mLock")
void removeExactAlarmsOnPermissionRevokedLocked(int uid, String packageName, boolean killUid) {
if (isExemptFromExactAlarmPermission(uid)
void removeExactAlarmsOnPermissionRevoked(int uid, String packageName, boolean killUid) {
if (isExemptFromExactAlarmPermissionNoLock(uid)
|| !isExactAlarmChangeEnabled(packageName, UserHandle.getUserId(uid))) {
return;
}
@@ -3891,7 +3890,9 @@ public class AlarmManagerService extends SystemService {
final Predicate<Alarm> whichAlarms = a -> (a.uid == uid && a.packageName.equals(packageName)
&& a.windowLength == 0);
removeAlarmsInternalLocked(whichAlarms, REMOVE_REASON_EXACT_PERMISSION_REVOKED);
synchronized (mLock) {
removeAlarmsInternalLocked(whichAlarms, REMOVE_REASON_EXACT_PERMISSION_REVOKED);
}
if (killUid && mConstants.KILL_ON_SCHEDULE_EXACT_ALARM_REVOKED) {
PermissionManagerService.killUid(UserHandle.getAppId(uid), UserHandle.getUserId(uid),
@@ -4807,10 +4808,7 @@ public class AlarmManagerService extends SystemService {
case REMOVE_EXACT_ALARMS:
int uid = msg.arg1;
String packageName = (String) msg.obj;
synchronized (mLock) {
removeExactAlarmsOnPermissionRevokedLocked(uid, packageName, /*killUid = */
true);
}
removeExactAlarmsOnPermissionRevoked(uid, packageName, /*killUid = */true);
break;
case EXACT_ALARM_DENY_LIST_PACKAGES_ADDED:
handleChangesToExactAlarmDenyList((ArraySet<String>) msg.obj, true);
@@ -4826,10 +4824,7 @@ public class AlarmManagerService extends SystemService {
uid = msg.arg1;
if (!hasScheduleExactAlarmInternal(packageName, uid)
&& !hasUseExactAlarmInternal(packageName, uid)) {
synchronized (mLock) {
removeExactAlarmsOnPermissionRevokedLocked(uid,
packageName, /*killUid = */false);
}
removeExactAlarmsOnPermissionRevoked(uid, packageName, /*killUid = */false);
}
break;
case CHECK_EXACT_ALARM_PERMISSION_ON_FEATURE_TOGGLE:
@@ -4849,10 +4844,7 @@ public class AlarmManagerService extends SystemService {
if (defaultDenied) {
if (!hasScheduleExactAlarmInternal(pkg, uid)
&& !hasUseExactAlarmInternal(pkg, uid)) {
synchronized (mLock) {
removeExactAlarmsOnPermissionRevokedLocked(uid, pkg,
true);
}
removeExactAlarmsOnPermissionRevoked(uid, pkg, true);
}
} else if (hasScheduleExactAlarmInternal(pkg, uid)) {
sendScheduleExactAlarmPermissionStateChangedBroadcast(pkg,

14
services/tests/mockingservicestests/src/com/android/server/alarm/AlarmManagerServiceTest.java
View File

@@ -2758,7 +2758,7 @@ public class AlarmManagerServiceTest {
mService.handleChangesToExactAlarmDenyList(new ArraySet<>(packages), false);
// No permission revoked.
verify(mService, never()).removeExactAlarmsOnPermissionRevokedLocked(anyInt(), anyString(),
verify(mService, never()).removeExactAlarmsOnPermissionRevoked(anyInt(), anyString(),
anyBoolean());
// Permission got granted only for (appId1, userId2).
@@ -2813,14 +2813,14 @@ public class AlarmManagerServiceTest {
mService.handleChangesToExactAlarmDenyList(new ArraySet<>(packages), true);
// Permission got revoked only for (appId1, userId2)
verify(mService, never()).removeExactAlarmsOnPermissionRevokedLocked(
verify(mService, never()).removeExactAlarmsOnPermissionRevoked(
eq(UserHandle.getUid(userId1, appId1)), eq(packages[0]), eq(true));
verify(mService, never()).removeExactAlarmsOnPermissionRevokedLocked(
verify(mService, never()).removeExactAlarmsOnPermissionRevoked(
eq(UserHandle.getUid(userId1, appId2)), eq(packages[1]), eq(true));
verify(mService, never()).removeExactAlarmsOnPermissionRevokedLocked(
verify(mService, never()).removeExactAlarmsOnPermissionRevoked(
eq(UserHandle.getUid(userId2, appId2)), eq(packages[1]), eq(true));
verify(mService).removeExactAlarmsOnPermissionRevokedLocked(
verify(mService).removeExactAlarmsOnPermissionRevoked(
eq(UserHandle.getUid(userId2, appId1)), eq(packages[0]), eq(true));
}
@@ -2833,7 +2833,7 @@ public class AlarmManagerServiceTest {
mIAppOpsCallback.opChanged(OP_SCHEDULE_EXACT_ALARM, TEST_CALLING_UID, TEST_CALLING_PACKAGE);
assertAndHandleMessageSync(REMOVE_EXACT_ALARMS);
verify(mService).removeExactAlarmsOnPermissionRevokedLocked(TEST_CALLING_UID,
verify(mService).removeExactAlarmsOnPermissionRevoked(TEST_CALLING_UID,
TEST_CALLING_PACKAGE, true);
}
@@ -2919,7 +2919,7 @@ public class AlarmManagerServiceTest {
null);
assertEquals(6, mService.mAlarmStore.size());
mService.removeExactAlarmsOnPermissionRevokedLocked(TEST_CALLING_UID, TEST_CALLING_PACKAGE,
mService.removeExactAlarmsOnPermissionRevoked(TEST_CALLING_UID, TEST_CALLING_PACKAGE,
true);
final ArrayList<Alarm> remaining = mService.mAlarmStore.asList();